Skip to content
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

Refactored XAML x:Names #433

Merged
merged 4 commits into from
Apr 18, 2019

Conversation

LanceMcCarthy
Copy link
Contributor

Fixes #62

Description of the changes:

-Refactored all x:Names to use the generally accepted Pascal-Casing

How changes were validated:

Unit Tests
Manual Tests

@LanceMcCarthy
Copy link
Contributor Author

@HowardWolosky Here's the new PR, from branch specifically for the issue.

Once this is merged, I'll make sure my fork's master is up to date and will always make sure I don't commit to master by accident (note to self - don't do this stuff late at night).

Copy link

@danbelcher-MSFT danbelcher-MSFT left a comment

Choose a reason for hiding this comment

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

@LanceMcCarthy, this PR includes commits that are unrelated. Please rebase your branch onto recent master. You can run:

> git fetch upstream master
> git checkout Issue62-Squashed
> git rebase -i upstream/master

In the editor, remove all lines that aren't your commits and squash the remaining commits using the 'fixup' command. From your branch, I ended up with the content:

pick f6848a5 Completed View (Calculator.xaml)
fixup 1b613ff Completed View (Calculator.xaml)

Save and close the file. The rebase should occur. Then run:

> git push -f Issue62-Squashed

@LanceMcCarthy
Copy link
Contributor Author

LanceMcCarthy commented Apr 7, 2019

@danbelcher-MSFT Thanks for the guidance to remove the commits. However, I couldn't rebase due to a conflict. For now, I've aborted git rebase -i upstream/master until I better understand what's happening because I don't want to make this worse :)

I followed your guidance and did the rebase with the following git-todo setup:'

image

When continuing, it results in the following:

image

Should I mark them as resolved and force the merge? I'm horrible with git from the command line.

@danbelcher-MSFT
Copy link

When git attempts the rebase, it tells you there are merge conflicts. Run git status and it will tell you which files have conflicts.

> git status
interactive rebase in progress; onto af41a18
Last command done (1 command done):
   pick f6848a5 Completed View (Calculator.xaml)
Next command to do (1 remaining command):
   fixup 1b613ff Completed View (Calculator.xaml)
  (use "git rebase --edit-todo" to view and edit)
You are currently rebasing.
  (fix conflicts and then run "git rebase --continue")
  (use "git rebase --skip" to skip this patch)
  (use "git rebase --abort" to check out the original branch)

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

        modified:   src/Calculator/Controls/CalculationResult.cpp
        modified:   src/Calculator/Controls/OverflowTextBlock.cpp
        modified:   src/Calculator/Views/Calculator.xaml
        modified:   src/Calculator/Views/Calculator.xaml.cpp
        modified:   src/Calculator/Views/CalculatorProgrammerDisplayPanel.xaml
        modified:   src/Calculator/Views/CalculatorProgrammerDisplayPanel.xaml.cpp
        modified:   src/Calculator/Views/CalculatorProgrammerOperators.xaml
        modified:   src/Calculator/Views/CalculatorProgrammerOperators.xaml.cpp
        modified:   src/Calculator/Views/CalculatorProgrammerRadixOperators.xaml
        modified:   src/Calculator/Views/CalculatorProgrammerRadixOperators.xaml.cpp
        modified:   src/Calculator/Views/CalculatorScientificAngleButtons.xaml
        modified:   src/Calculator/Views/CalculatorScientificAngleButtons.xaml.cpp
        modified:   src/Calculator/Views/CalculatorScientificOperators.xaml
        modified:   src/Calculator/Views/CalculatorScientificOperators.xaml.cpp
        modified:   src/Calculator/Views/CalculatorStandardOperators.xaml
        modified:   src/Calculator/Views/HistoryList.xaml
        modified:   src/Calculator/Views/MainPage.xaml
        modified:   src/Calculator/Views/NumberPad.xaml
        modified:   src/Calculator/Views/NumberPad.xaml.cpp
        modified:   src/Calculator/Views/UnitConverter.xaml
        modified:   src/Calculator/Views/UnitConverter.xaml.cpp

Unmerged paths:
  (use "git reset HEAD <file>..." to unstage)
  (use "git add <file>..." to mark resolution)

        both modified:   src/Calculator/App.xaml

So I can see that App.xaml has a conflict. Open the file and resolve the conflict, then save the file. Then:

> git add src/Calculator/App.xaml
> git rebase --continue

And you keep following this process until all conflicts are resolved and the rebase can be completed. Then force push your branch as before.

@HowardWolosky HowardWolosky added codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) needs author feedback labels Apr 12, 2019
@danbelcher-MSFT
Copy link

ping @LanceMcCarthy. Are we taking this change still?

@LanceMcCarthy
Copy link
Contributor Author

Ugh, notification got lost in the noise, sorry. Will attempt this tonight.

Completed View (CalculatorProgrammerDisplayBoard)

Completed View (CalculatorProgrammerOperators)

Completed View (CalculatorProgrammerRadixOperators.xaml)

Completed View (CaclulatorScientificAngleButtons)

Completed View (CaclulatorScientificOperators.xaml)

Completed View (CalculatorStandardOperators)

Completed View (HistoryList)

Completed View (MainPage)

Completed View (NumberPad)

Completed View (UnitConverter)

Cleared all remaining camel-case x:Names
@LanceMcCarthy
Copy link
Contributor Author

@danbelcher-MSFT Hopefully I got it this time. Was able to manually handle the conflicts, there was some new stuff in there that wasn't really a conflict, just on differnet lines or added properties.

Copy link

@danbelcher-MSFT danbelcher-MSFT left a comment

Choose a reason for hiding this comment

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

Lance, thanks so much! This definitely improves the consistency of our XAML pages and their code-behind. Thanks for being patient through all the churn.

@danbelcher-MSFT danbelcher-MSFT merged commit 8520d3f into microsoft:master Apr 18, 2019
@LanceMcCarthy LanceMcCarthy deleted the Issue62-Squashed branch April 18, 2019 23:05
EriWong pushed a commit to EriWong/calculator that referenced this pull request Jun 5, 2019
Description of the changes:
-Refactored all x:Names to use the generally accepted Pascal-Casing

How changes were validated:
Unit Tests
Manual Tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) needs author feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Calc x:Name values to begin with uppercase
3 participants