Conversation
joemull
left a comment
There was a problem hiding this comment.
Been a while since I've dealt with reverts, but I believe what we have here is the right kind of building blocks, but they point to the wrong hashes.
Here are the 4 problem commits on master:
04b1c28bb (HEAD -> master, origin/master, origin/HEAD) a11y: #5105 change credit link to read more
a78d1fb11 bugfix: credit displaying twice on OLH article
c99de767e bugfix: credit not displaying on OLH article
5681a4dee a11y: #5105 update credit link markup for screen-readers
And here are the 8 commits on this branch:
3078fac66 (HEAD -> b-5105-revert, origin/b-5105-revert) Revert "a11y: #5105 update credit link markup for screen-readers"
6208dd63f Revert "bugfix: credit not displaying on OLH article"
77bd82429 Revert "bugfix: credit displaying twice on OLH article"
7d0826dd8 Revert "a11y: #5105 change credit link to read more"
2fc5e3c86 (origin/b-5105-olh-article-icon-aria, b-5105-olh-article-icon-aria) a11y: #5105 change credit link to read more
7df7f8867 bugfix: credit displaying twice on OLH article
d4ce7c464 bugfix: credit not displaying on OLH article
fa52894c3 a11y: #5105 update credit link markup for screen-readers
Notice two things:
- There PR branch is picking up 8 commits in the diff, which shows you something isn't right--it should only pick up the reversion commits, not the targets / originals.
- The hashes on the original 4 commits differ between the PR branch and master.
The solution is to check out master, create a new branch, and then revert the commits that are on master. Then check there are only 4 commits in the PR diff.
The reason the hashes differ between the old branch and master is rebasing. We click "Merge and rebase", which means GitHub reflows the branch and generates new hashes for each commit.
3078fac to
da41315
Compare
|
yes, you're right, I worked off the original branch - and it should have been from master. I think I've now pushed the right set of reverts! |
While working on the original #5106 I had some unresolved concerns #5106 (comment)
Further discussion of #5106 with @joemull led us to realise this was addressing an issue on master that has already been fixed in 1.8 branch, and that fix introduced a new template such that these changes will conflict and be superceeded.
The simplest way forward at this point seems to be to revert these changes, and wait for 1.8 changes to be brought into master.
Also drawing this to the attention of @ajrbyers as the other original reviewer.