Skip to content

/ Unity

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor views to update settings in MaybeUpdateData #275

Merged
merged 7 commits into from Sep 8, 2017
Merged

Refactor views to update settings in MaybeUpdateData #275

merged 7 commits into from Sep 8, 2017

Conversation

@StanleyGoldman
Copy link
Member

StanleyGoldman commented Sep 5, 2017

No description provided.

@StanleyGoldman
Updating SettingsView to get metrics enabled state in MaybeUpdateData
f86e290
@StanleyGoldman
Updating BranchesView to get favorite branches in MaybeUpdateData
Loading status checks…
b1b9af2
@StanleyGoldman StanleyGoldman mentioned this pull request Sep 5, 2017
Merged
1 of 1 task complete
@StanleyGoldman
Unintentional change
bbc4153
@StanleyGoldman
Adding new fields in a different place to make merging easier
Loading status checks…
676e8ff
@StanleyGoldman StanleyGoldman mentioned this pull request Sep 5, 2017
Merged
2 of 2 tasks complete
shana added 2 commits Sep 8, 2017
@shana
Remove redundant overload and simplify comparison
Loading status checks…
cb3b60f 
Other places in the code call IsFavourite with other types of objects
that aren't strings (like a GitBranch instance). We either have overloads
for all of them or we just take a string and the caller should know
what to pass us, less of an API footprint to worry about.
@shana
Merge master into fixes/settings-update-on-enable
Loading status checks…
a4b0664
@shana
shana reviewed Sep 8, 2017
View changes
src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs Outdated
public override void InitializeView(IView parent)
{
base.InitializeView(parent);
usageTracker = Manager != null && Manager.UsageTracker != null ? Manager.UsageTracker : null;

This comment has been minimized.

Sign in to view
@shana

shana Sep 8, 2017

Contributor

This is not needed, the Manager is always guaranteed to exist

This comment has been minimized.

Sign in to view
@shana

shana Sep 8, 2017

Contributor

UsageTracker is never null either so we don't have to worry about that
@shana
Manager and UsageTracker are never null so we don't have to worry abo…
Loading status checks…
db45ff1 
…ut it
@shana
shana approved these changes Sep 8, 2017
View changes
@shana shana merged commit 7bbc76f into master Sep 8, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@shana shana deleted the fixes/settings-update-on-enable branch Sep 8, 2017
@StanleyGoldman StanleyGoldman mentioned this pull request Sep 11, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shana shana

Assignees
No one assigned
Labels
ready-for-review
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@StanleyGoldman @shana
You can’t perform that action at this time.