Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

Merge conflicts in release notes #2925

Closed
kodebach opened this issue Sep 1, 2019 · 11 comments
Closed

Merge conflicts in release notes #2925

kodebach opened this issue Sep 1, 2019 · 11 comments
Assignees

Comments

@kodebach
Copy link
Member

kodebach commented Sep 1, 2019

The way we currently write our release notes is prone to merge conflicts.

Here is a list of solutions I could think of/find online (some from #2923):

  • Add more <<TODO>> placeholders and instruct people to choose a random placeholder instead of the first. This makes merge conflicts less likely, but depends on people actually choosing somewhat randomly. A side effect of this would be that a merge conflict could be solved by a simple commit changing, which placeholder was replaced, instead of a rebase.
  • Get rid of the placeholders and use a custom merge driver. Commonly the union merge driver is suggested for this. However, it is very problematic, if there are modified lines as well as added lines in the merge conflicts. (It may duplicate lines without the user noticing).
  • The one completely safe solution I could think of was to have one release notes file for each PR. A script could then be used to merge them before a release. There would have to be some mechanism for updating release notes from previous PRs.
@markus2330
Copy link
Contributor

Thank you for making this proposal!

It may duplicate lines without the user noticing

This is no problem at all, as before the release I deduplicate the release notes anyway.

I think we can use the union merge driver for the link checker, icheck and the release notes.

@Chemin1 what do you think about this? I actually was thinking to use a union merge also for comments in config files (maybe with some heuristic to detect typo-fixes. Does git provide such a merge driver?)

@dominicjaeger
Copy link
Contributor

git can use the built-in union merge driver with an attribute. It is not a merge strategy on its own (I've never used it btw). Is this the driver that you meant here

Commonly the union merge driver is suggested for this.

@kodebach ?

before the release I deduplicate the release notes anyway.

In this case using the union driver looks pretty straightforward to me.

I actually was thinking to use a union merge also for comments in config files

I have thought about this, too, but was not sure how we would like to have this implemented exactly. Thus, I've now opened a new issue (it is quite long) for this topic.

@kodebach
Copy link
Member Author

kodebach commented Sep 2, 2019

Is this the driver that you meant here

Yes, git calls this attribute "merge driver".

before the release I deduplicate the release notes anyway.

In this case using the union driver looks pretty straightforward to me.

I agree, if there is a manual review anyway, it shouldn't be a problem.

@markus2330
Copy link
Contributor

@Chemin1: can you implement this for our 3 files?

@dominicjaeger
Copy link
Contributor

@Chemin1: can you implement this for our 3 files?

Yes :)

@markus2330
Copy link
Contributor

@Chemin1: any progress here?

Such merge conflicts happen quite often (e.g. right before in #3002) and take a lot of time to fix for beginners.

@dominicjaeger
Copy link
Contributor

If we take for example the one from #3002:

<<<<<<< master
- We updated links for the INI parsing library Nickel. _(René Schwaiger)_
- Added a new [Get Started](../../doc/GETSTARTED.md). _(Hani Torabi)_
- Added some informations about [contributing](../../.github/CONTRIBUTING.md) to Elektra. _(Hani Torabi)_
=======
- Added design decision for error code implementations. _(Michael Zronek)_
>>>>>>> master

Does anyone have a better idea than "Take everything from both versions"?

@kodebach
Copy link
Member Author

kodebach commented Nov 5, 2019

The one completely safe solution I could think of was to have one release notes file for each PR. A script could then be used to merge them before a release. There would have to be some mechanism for updating release notes from previous PRs.

That's the only solution I could think of that circumvents merge conflicts. It would make development easier for beginners, but complicates the release process (merging the various files could be difficult). It also causes problems, if multiple PRs implement one feature in several parts. Ideally such PRs would share one release notes file.

@dominicjaeger
Copy link
Contributor

dominicjaeger commented Nov 5, 2019

That's the only solution I could think of that circumvents merge conflicts.

You mean it circumvents conflicts when creating the release note file for a PR, not generally, right?

merging the various files could be difficult

=> We essentially moved the conflicts to a later moment?

Update: I do not know how to integrate Elektra into GitHub to automatically to resolve conflicts in exactly this file. But with @kodebach's approach someone could call cmerge-config-files multiple times with a strategy that uses LibGit2's union merge before releasing.

@markus2330
Copy link
Contributor

markus2330 commented Nov 5, 2019

We already agreed that we can simply keep everything in this file (take ours and theirs).

We essentially moved the conflicts to a later moment?

Before release someone needs to polish the whole text anyway.

cmerge-config-files multiple times with a strategy that uses LibGit2's union merge before releasing

I think you can simply use the union merge which already exists in git. Using merge-config-files would be a nice test how well it works, though.

@dominicjaeger
Copy link
Contributor

#3163 seems to work according to @markus2330.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants