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

Adding a change log #225

Merged
merged 19 commits into from
Jul 21, 2021
Merged

Adding a change log #225

merged 19 commits into from
Jul 21, 2021

Conversation

IshaanDesai
Copy link
Member

Adding a CHANGELOG to the tutorials repository.

@IshaanDesai IshaanDesai self-assigned this Jul 14, 2021
@IshaanDesai IshaanDesai requested review from MakisH and uekerman July 14, 2021 10:56
@MakisH
Copy link
Member

MakisH commented Jul 15, 2021

Looks good: it looks like you followed the format used in preCICE and you transferred the release notes (have not checked in detail).

Just remember to contribute the changelog entries as independent files, to avoid merge conflicts.

@IshaanDesai
Copy link
Member Author

Just remember to contribute the changelog entries as independent files, to avoid merge conflicts.

I dont really understand what you are saying here, can you please explain more 😅

@davidscn
Copy link
Member

Have a look at the preCICE changelogs: https://github.com/precice/precice/tree/develop/docs/changelog

The CHANGELOG file is usually only updated for new version release. The changelog entries are collected in separate files in order to avoid merge conflicts.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@IshaanDesai
Copy link
Member Author

Have a look at the preCICE changelogs: https://github.com/precice/precice/tree/develop/docs/changelog

The CHANGELOG file is usually only updated for new version release. The changelog entries are collected in separate files in order to avoid merge conflicts.

Okay, so this would only be valid for changes done from the last release till now correct? And how do we decide the numbers in the names of the files? Would the merge conflicts happen because multiple feature developments would consecutively edit the CHANGELOG? In that case does it not work if every feature developer just pulls, resolves the conflict and pushes to the CHANGELOG?

@IshaanDesai IshaanDesai requested review from MakisH and uekerman July 15, 2021 11:17
@davidscn
Copy link
Member

Have a look at the preCICE changelogs: https://github.com/precice/precice/tree/develop/docs/changelog
The CHANGELOG file is usually only updated for new version release. The changelog entries are collected in separate files in order to avoid merge conflicts.

Okay, so this would only be valid for changes done from the last release till now correct? And how do we decide the numbers in the names of the files? Would the merge conflicts happen because multiple feature developments would consecutively edit the CHANGELOG? In that case does it not work if every feature developer just pulls, resolves the conflict and pushes to the CHANGELOG?

The numbers in the file names correspond to the PR number of the repo. To your second questions: yes that's right and yes that would be done as an alternative.

@fsimonis
Copy link
Member

In that case does it not work if every feature developer just pulls, resolves the conflict and pushes to the CHANGELOG?

Yes, but this results in a merge conflict for each change since the PR started. Most importantly, it is so annoying that devs become angry. And angry devs are not got at resolving merge conflicts without accidentally deleting old entries.

The steps are easy:

  • define a dir that doesn't break the jekyll import script.
  • Use a script to create the changelong example
  • Use a script to generate new entries using gh-cli example

Copy link
Member

@uekerman uekerman left a comment

Choose a reason for hiding this comment

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

Some c&p problems I guess. Otherwise good to go from my side.
I am not sure about the markdown lint things.

Do we want to have the changelog-entries under docs?

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@IshaanDesai
Copy link
Member Author

Do we want to have the changelog-entries under docs?

I now have the entries at the base directory

.markdownlint.json Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@IshaanDesai IshaanDesai requested a review from MakisH July 21, 2021 09:23
@IshaanDesai
Copy link
Member Author

The failing workflows are not relevant to the addition of the CHANGELOG and hence need to be investigated independently.

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

I noticed 2-3 small things. Please consider them and then feel free to merge!

.github/workflows/check-markdown.yml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
- Added new volume coupled diffusion tutorial with FEniCS [#219](https://github.com/precice/tutorials/pull/219)
Copy link
Member

Choose a reason for hiding this comment

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

Just FYI: this seems to be unrelated. But if this is coming from another branch, then it is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is indeed a change coming from another PR but as the whole change-log system is implemented here, it has been added via this PR.

@IshaanDesai IshaanDesai merged commit c4ef89e into develop Jul 21, 2021
@IshaanDesai IshaanDesai deleted the add-changelog branch July 26, 2021 05:59
@BenjaminRodenberg BenjaminRodenberg added this to the v202104.2.0 milestone Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants