-
Notifications
You must be signed in to change notification settings - Fork 685
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
Fixes #3522. Adds IDesignable
: Ability for Views to load design-time/demo data
#3575
Conversation
I am doing rebase wrong, clearly. I had to re-create this PR by manually copying my changes from #3568 because it was screwed up somehow. I have now done this 3-4 times and I have no clue what I'm doing wrong. |
I'll take a quick look at the current state of the branch and see if I can piece together what went wrong. No promises without the original commits still existing, though, which is pretty unlikely unless you went out of your way to keep doing everything on new branches with each push. |
Taking a look at the commit graph, my first suspicion is those merges. Dollars to donuts I'd bet those are at least a significant part of the difficulty, because they move the common parent to a place where you're probably going to get strange things like merge conflicts that aren't actually any different between the two copies or duplicate commits, or other annoying/strange junk like that. That gets worse and worse if other merge activity occurs to the branch you branched from (so v2_develop), that you're trying to rebase your work branch on, after you've also merged from v2_develop into your working branch. It makes a diamond dependency, sorta (it's more that it makes the v2_develop branch an ancestor of itself via your branch, which looks to the graph like jumping back in time, yet moving forward at the same time), and it just tries its best to replay the deltas for you. But especially if you're using the default git diff algorithm, that gets......weird..... Setting the diff/merge algorithm to histogram sometimes can help a little bit, at least in making the diffs make a bit more sense, but the solution is to never merge outward. Outward meaning from a branch closer to the root to a branch farther from root. In general, once you branch, the only merges that branch should ever be involved in are merges into an ancestor with a common base that is no older than the original branch point, or as the recipient of merges from descendent branches of itself. To "merge" outward (so, to apply changes from an ancestor branch to your working branch) you rebase, effectively changing the branch point, which replays the diffs on the new base, giving you the opportunity to get involved if there are commits it can't auto-apply. When doing a rebase, an interactive rebase is a good opportunity to squash, drop, or otherwise modify commits of your working branch, since you're already re-writing history for your branch. However, if the branch you're working on has descendents, do not force push a rebase over it, because you will orphan the other branch, which can be a bitch to fix sometimes because there's no longer a common base. Push to a new branch, instead, if there's a descendent branch. You can actually always push to a new branch, after a rebase, if you want to preserve the old history for any reason. You would just have to update the PR or create a new one if you did that. But yeah... Anything that pulls in commits from an ancestor before you merge back into that ancestor has the potential (not a guarantee) to create conflicts and other fun. You can often get away with it, especially if you didn't touch any of the same files, but the nature of merges means that, when it does bite you, it's usually going to bite harder than your average merge conflict. |
A thought I just had that I've never tried but sounds valid in my head is that, if you have done outward merges like that... If you do an interactive rebase (if it'll let you), you could try to drop the commits for the merges from the ancestor branch into your branch. It should work, even if your code is dependent on what was merged in, because that's all already there, already, at the new ref you're rebasing on. Then, your branch will no longer make its ancestor its own grandpa. 🤷♂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking a look at it beyond what we were exploring in the associated issue, around nitpicky stuff with generics.
First thing that comes to mind, just scanning the file list: Are those all the places you wanted to have this, for now (ie no other view types)?
Looking at the actual code now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
A couple of super minor opportunistic suggestions, but good to go as-is if you don't want to bother.
You are confusing two issues.
Any view that could show meaningful state could support this. There's no pressing need to hit them all now. I imagine when we get to , for example, we'll want to add this to it to help development/testing. |
Back when I simply did Here is my typical workflow. When I want to create a new PR to address an issue I do git checkout v2_develop
git pull upstream v2_develop
# push to my fork if needed, so it matches
git push
# create new branch
git checkout -b v2_xxx-branch Then I work, and eventually do git add . ; git commit -m "commit msg"
git push And I use github.com to create the remote Then, if, as I'm working, git checkout v2_develop
git pull upstream v2_develop
git push
git checkout v2_xxx-branch
#double check I've pushed my latest and have the latest of my branch
git push
git pull origin v2_xxx-branch
git rebase --onto v2_develop
# pull branch down and re-apply changes
git pull
# fix any merge issues
git mergetool
# commit
git add . ; git commit -m "rebased with v2_develop" This never feels right, so whatever I'm doing wrong is in the stuff around |
I think the problem is in the |
Sorry, ignore the local commits not yet pushed because you already do a |
Maybe the culprit is the |
I suspected I might be.
Sounds like a good plan to me. 👍 |
Here's what I do when working on a github repo that I have a fork of, that helps avoid unexpected weirdness:
|
If I drop or squash anything during the rebases above, I sometimes will push a new branch, to preserve the old one, just in case, and if I remember to do so. |
This step:
Has the potential to cause a merge that could cause unexpected annoyances, since the default is to merge from your remote into your local, if they differ. You can change that behavior default to rebase (what I have set as the default behavior) or to fast-forward only, if you want, or you can specify it at pull-time with --rebase=[base] or --ff-only. Both will avoid a merge. --ff-only will fail if fast-forward isn't possible, which is usually a good thing in this use case, since it means history diverged. However, a git checkout already does a pull. It's a shortcut for a If I do it at the command line, I usually use |
That step stood out to me, too, because of the implicit merge that happens if history differs. Check out the steps I posted a couple comments up from here for what I do that avoids any of these issues while using the recommended rebase/branch outward, merge inward flow. I most often use GitKraken to interact with it, but it respects your git configuration and also can show you exactly what it did to achieve an operation, which is part of how I originally learned pieces of how it all works, a while back. Pretty sure that's still available with a free license, and it's available on all platforms. I use it in windows and linux, and it's just...NICE. |
Fixes
Proposed Changes/Todos
Slider
RadioGroup
ListView
MenuBar
Pull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commit///
style comments)