Sitelet https://github.com/DragonBox/uplift/pull/39
Skip to content

Fix upfile not refreshed before install - #39

Merged
niezbop merged 1 commit into
DragonBox:masterfrom
niezbop:fix/refresh_upfile_before_install
Dec 11, 2017
Merged

niezbop merged 1 commit into
DragonBox:masterfrom
niezbop:fix/refresh_upfile_before_install

Conversation

@niezbop

@niezbop niezbop commented Dec 8, 2017

Copy link
Copy Markdown
Member

Fixes #38

@lacostej

Copy link
Copy Markdown
Member

Not really happy with committing this without understanding how that would fix the problem. The only explanation I have is if this indeed changes the behavior is that named parameters are sort of ignored with the compiler we use.

@niezbop

niezbop commented Dec 11, 2017 •

Copy link
Copy Markdown
Member Author

That's not an issue with the named parameters, but an issue with the singleton nature of the UpliftManager and the way ResetInstances is implemented:

public static void ResetInstances()
{
    instance = null;
    Upfile.ResetInstance();
    Upbring.ResetInstance();
    InitializeInstance();
}

internal static void InitializeInstance()
{
    instance = new UpliftManager();
    instance.upfile = Upfile.Instance();
}

Because we have instance = null;, when we try to refresh the instances from an instance of the UpliftManager, it simply does nothing. The static instance of the UpliftManager will have the up-to-date Upfile and Upbring, but the instance that called the ResetInstances, as we did before this commit, will not have its updated Upfile.

I removed the name parameter because it simply could not work in the current implementation of the UpliftManager, and therefore had no point of being in the code (for now).

@lacostej
lacostej self-requested a review December 11, 2017 10:23

@lacostej lacostej left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Great.

@niezbop
niezbop merged commit 3b62c5f into DragonBox:master Dec 11, 2017
@niezbop
niezbop deleted the fix/refresh_upfile_before_install branch December 11, 2017 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants