Skip to content

Test PR after line ending normalization#12864

Closed
seanbudd wants to merge 1 commit into
test-normalized-line-endingsfrom
test-branch-from-master
Closed

Test PR after line ending normalization#12864
seanbudd wants to merge 1 commit into
test-normalized-line-endingsfrom
test-branch-from-master

Conversation

@seanbudd

@seanbudd seanbudd commented Sep 21, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

This PR is a test sample for #12387
This PR simulates an open PR to master, of a branch created before normalization, after normalization has occurred on master.

In this case test-normalized-line-endings has had all line endings changed to LF using the process in #12816 - this simulates origin/master after normalization.
test-branch-from-master has been branched from master, without normalization, and has had a small change applied to a file with CRLF line endings.

This shows that line ending normalization will cause most current PRs to have merge conflicts.

As a strategy to handle this, branches should merge master in the following way.
This example uses these test branches, so that devs can confirm the process locally.

  1. Set the merge strategy to renormalize for the repository: 
    git config --local merge.renormalize true
  2. Existing merge conflicts with master will still need to be resolved. See #PRTBA 
    git merge origin/test-normalize-line-endings # To be replaced with origin/master

In summary, the final script will look something like

git config --local merge.renormalize true
For each pr:
  gh pr checkout pr.num
  git fetch origin/master
  git merge origin/master
  # if the merge is successful, push, else ?

Message for contributors

NVAccess has updated the NVDA repository to make the line endings normalized to LF. 
This created merge conflicts, which we have resolved using the methodology discussed in #12864.
For more information see the following:
* Project overview: #12387
* Detailed implementation and justification: #12816.

@seanbudd seanbudd requested a review from a team as a code owner September 21, 2021 00:12
@seanbudd seanbudd requested review from michaelDCurran and removed request for a team September 21, 2021 00:12
@seanbudd seanbudd marked this pull request as draft September 21, 2021 00:12
@seanbudd seanbudd removed the request for review from michaelDCurran September 21, 2021 00:17
This was referenced Sep 21, 2021
Merged
Closed
@feerrenrut

feerrenrut commented Sep 21, 2021

Copy link
Copy Markdown
Contributor

I think we'll have to update all the PR's ourselves. Additionally, can you prepare an explainer comment that can be posted onto each PR?

It might be possible to script this with gh CLI tool.

@seanbudd

seanbudd commented Sep 21, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

@feerrenrut - What are your thoughts on the following?

NVAccess has updated the NVDA repository to make the line endings normalized to LF.
This created merge conflicts, which we have resolved using the methodology discussed in #12864.
For more information see the following:

This might help to make suggestions:

- NVAccess has updated the NVDA repository to make the line endings normalized to LF. 
- This created merge conflicts, which we have resolved using the methodology discussed in #12864.
- For more information see the following:
- * Project overview: #12387
- * Detailed implementation and justification: #12816.
+ NVAccess has updated the NVDA repository to make the line endings normalized to LF. 
+ This created merge conflicts, which we have resolved using the methodology discussed in #12864.
+ For more information see the following:
+ * Project overview: #12387
+ * Detailed implementation and justification: #12816.

I'll look into a script to perform this for all PRs.

@seanbudd

seanbudd commented Sep 21, 2021

Copy link
Copy Markdown
Member Author

I have a major concern - how do we handle PRs that have merge conflicts currently?
I think instead the script will have to be something like this, and we will have to handle the merge conflicts manually.

git checkout origin/master .gitattributes
git add --renormalize .
git commit -m "normalize line endings"

@feerrenrut

feerrenrut commented Sep 21, 2021

Copy link
Copy Markdown
Contributor

When a PR has merge conflicts, either us or the author is expected to resolve. There probably needs to be a manual step to confirm that the merge went correctly.

@seanbudd

seanbudd commented Sep 21, 2021

Copy link
Copy Markdown
Member Author

I don't think our script would work in this case, and we will not be able to merge in master to resolve conflicts created from the normalization.

@seanbudd

seanbudd commented Sep 21, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

If we set git config --local merge.renormalize true we can perform these merges without conflicts directly.

@seanbudd seanbudd closed this Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@seanbudd @feerrenrut