[INFRA] Autogenerate CHANGES.md #103

franklin-feingold · 2018-12-06T01:57:17Z

PR will close #3

This solution will collect the merged PR and put it under the unreleased section. It will also update the unreleased section to the new release number after the release has occurred when a new PR has been merged (the next one after the release PR, with that one now being under the unreleased section). The src/CHANGES.md is how it looks ran on this repo in the current state.

The changelog will only generate and push when there is a merged PR (it will not push on a release PR or regular commits). A release PR will contain one change (changing the Unreleased title to x.x.x [the version number])

The changelog has also been added to mkdocs. Some of the specification documentation was updated to pass the updated .remarkrc file.

To complete the addition - a github token needs to be added to the CircleCI environmental variables named CHANGE_TOKEN. I can do that if this solution looks good before the merge.

update 12/4

chrisgorgo · 2018-12-08T17:53:06Z

.circleci/config.yml

+             mv ~/build/tmp/CHANGES.md ~/build/src/CHANGES.md
+             git config credential.helper 'cache --timeout=120'
+             git config user.email "franklin.feingold@gmail.com"
+             git config user.name "franklin-feingold"


Probably better to change it to something like "changelog bot".

I have renamed the job to "Changelog-bot" (unless I misinterpreted, I can change what you meant)

I think he meant change the name of user.name.

Thank you, I have committed the change

chrisgorgo · 2018-12-08T17:53:22Z

.remarkrc

+    ["lint-no-duplicate-headings", false],
+    ["lint-list-item-indent", "tab-size"],
+    ["lint-emphasis-marker", "consistent"],
+    ["lint-maximum-line-length", false]


Why are those changed to the linter necessary for the changelog generation?

Those changes are necessary for the auto generated changelog (after it was corrected through remark) to pass the linter style guide

chrisgorgo · 2018-12-08T17:55:24Z

.circleci/config.yml

+           command: |
+             if git log -1 --pretty=%s | grep Merge* ; then
+             if git log -1 --pretty=%b | grep release ; then
+             echo "New release, do nothing"


Is this check necessary? I wonder if there could be a situation where PR title would include the word "release" (with a different meaning).

For a better check, if it's a release tag, then git describe should be the same as the latest tag.

DESC=$(git describe --tags) test "$(git tag --list $DESC)" = "$DESC" # True if on the tag else won't match

This check is necessary to make sure that the changelog will only generate on merged PR's. It will skip for commits and release PRs. For a release PR title we could have a controlled vocabulary or set of words to denote only a new specification release ("New main specification release"? with the new version followed i.e. New main specification release 1.2.3). I agree there can be a situation where a PR title could include the word "release" in it and mean something else.

The release tag could work. The git describe --tags outputs the "latest release tag"-# of commits since release-g"(commit string)" . This can be parsed down to get the latest release tag, but I noticed that the git tag --list will output whatever tag you provide it, so the test will be true for all cases. I may not see how the test will skip the changelog generation for a new release?

Try in some project with tags (say 0.1.0):

for REF in 0.1.0 master; do git checkout -q $REF DESC=$(git describe --tags) if [ "$(git tag --list $DESC)" = "$DESC" ]; then echo "We're on a tag: $DESC" else echo "Not a tag, after all: $DESC" fi done

In Pybids, for example:

We're on tag: 0.1.0 Not a tag, after all: 0.6.5-229-gbd23eaa

This is because git tag --list 0.6.5-229-gbd23eaa doesn't match any tag, and produces an empty string.

Ah, I understand. Thank you for explaining this. My concern is that when circle is testing for on/off a tag, we will not get the desired result of 'We're on tag' after the merged PR denoting a new release. If you merge the PR then do a github release, it will tag the merged PR, but the circle workflow won't pick that up because the release happened after the merged PR. This would work if circle didn't run until the release was triggered, the merged PR would have the tag and then the test would notice to skip it. It does not appear one can tag an open PR.

Oh, I see. You plan to do release prep PRs. In which case, I would recommend using a template like "REL: " for the PR name (or similar), and then looking for the "REL: " string in the merge commit. Otherwise you'll have to be vigilant about any PRs that have "release" somewhere in the title, which may show up innocently. "REL: " is much less likely to happen by accident.

I have applied your "REL:" recommendation for the PR name into the config to lookout for and ignore running the changelog generator (and the commands downstream like remark). I also added a release protocol that states REL: needs to be in the PR name.

chrisgorgo · 2018-12-08T17:57:04Z

.circleci/config.yml

+             if git log -1 --pretty=%b | grep release ; then
+             echo "New release, do nothing"
+             mkdir ~/project/src/tmp
+             touch ~/project/src/tmp/empty.txt


This seems identical to the "not a pr merge commit" path - logic could be simplified.

Across the two jobs it is a similar path. The jobs have to be separated because of the docker environment differences between the two jobs.

simplified the logic into one if statement

chrisgorgo · 2018-12-08T17:58:17Z

Great work! Thanks for putting this together!

effigies

Overall looks reasonable. Some comments.

effigies · 2018-12-09T13:09:54Z

.circleci/config.yml

+           command: |
+             if git log -1 --pretty=%s | grep Merge* ; then
+             if git log -1 --pretty=%b | grep release ; then
+             echo "New release, do nothing"


For a better check, if it's a release tag, then git describe should be the same as the latest tag.

DESC=$(git describe --tags) test "$(git tag --list $DESC)" = "$DESC" # True if on the tag else won't match

effigies · 2018-12-10T15:29:13Z

.circleci/config.yml

+               - github-changelog-generator
+      - push:
+          requires:
+               - remark


Do you want to run this on every commit? I think limiting to branch: master will make more sense:

Suggested change

- remark

- remark

filters:

branches:

only: master

This should probably apply to github-changelog-generator and remark as well.

I have applied this change in my recent commit

effigies · 2018-12-10T15:32:22Z

.circleci/config.yml

+             git config user.name "franklin-feingold"
+             git add ~/build/src/CHANGES.md
+             git commit -m "autobuild CHANGES.md"
+             git push https://${CHANGE_TOKEN}@github.com/bids-standard/bids-specification.git master


I might suggest pushing to another branch than master, in case you want to modify before pushing to master. But if the plan is just to clean up later, then no big deal.

The cleaning up happens automatically so it should be all ready for the master branch (with no linter errors). The hope is that this is an automatic process not needing manual intervention. If the automation cannot be realized then pushing to another branch and then be PR'd over to master would work.

effigies · 2018-12-10T15:37:33Z

.circleci/config.yml

+             git config user.email "franklin.feingold@gmail.com"
+             git config user.name "franklin-feingold"
+             git add ~/build/src/CHANGES.md
+             git commit -m "autobuild CHANGES.md"


I would prefer a cleaner/more descriptive message. Something like [DOC] Auto-generate changelog entry for PR #XYZ.

I have applied this update to my recent commit

chrisgorgo · 2018-12-17T01:12:19Z

RTM @franklin-feingold ?

franklin-feingold · 2018-12-17T18:29:04Z

@chrisfilo This should be set! I added the environmental variable "CHANGE_TOKEN" to the circleCI environmental variables.

chrisgorgo · 2018-12-17T20:51:23Z

Many thanks!

franklin-feingold and others added 4 commits December 4, 2018 17:07

Merge pull request #7 from bids-standard/master

6268b23

update 12/4

Update for changelog PR

1736b6c

config.yml updated testing of updated CHANGES.md

397712d

edit directories in config.yml

54b2201

chrisgorgo suggested changes Dec 8, 2018

View reviewed changes

chrisgorgo requested a review from effigies December 8, 2018 17:58

effigies reviewed Dec 10, 2018

View reviewed changes

franklin-feingold and others added 3 commits December 10, 2018 18:18

Apply review comments 12/10

0b800d4

Change config user.name

a6fa2f4

Update config.yml and add release protocol

7f98073

chrisgorgo approved these changes Dec 17, 2018

View reviewed changes

chrisgorgo merged commit 80dd60a into bids-standard:master Dec 17, 2018

franklin-feingold added a commit that referenced this pull request Dec 17, 2018

[DOC] Auto-generate changelog entry for PR #103

dfdaf1c

sappelhoff changed the title ~~Autogenerate CHANGES.md~~ [INFRA] Autogenerate CHANGES.md Sep 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INFRA] Autogenerate CHANGES.md #103

[INFRA] Autogenerate CHANGES.md #103

franklin-feingold commented Dec 6, 2018

chrisgorgo Dec 8, 2018

franklin-feingold Dec 11, 2018

effigies Dec 11, 2018

franklin-feingold Dec 12, 2018

chrisgorgo Dec 8, 2018

franklin-feingold Dec 11, 2018

chrisgorgo Dec 8, 2018

effigies Dec 9, 2018

franklin-feingold Dec 11, 2018

effigies Dec 11, 2018

franklin-feingold Dec 12, 2018

effigies Dec 12, 2018

franklin-feingold Dec 15, 2018

chrisgorgo Dec 8, 2018

franklin-feingold Dec 11, 2018

franklin-feingold Dec 15, 2018

chrisgorgo commented Dec 8, 2018

effigies left a comment

effigies Dec 9, 2018

effigies Dec 10, 2018

franklin-feingold Dec 11, 2018

effigies Dec 10, 2018

franklin-feingold Dec 11, 2018

effigies Dec 10, 2018

franklin-feingold Dec 11, 2018

chrisgorgo commented Dec 17, 2018

franklin-feingold commented Dec 17, 2018

chrisgorgo commented Dec 17, 2018

-               - remark
+               - remark
+           filters:
+               branches:
+                   only: master

[INFRA] Autogenerate CHANGES.md #103

[INFRA] Autogenerate CHANGES.md #103

Conversation

franklin-feingold commented Dec 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisgorgo commented Dec 8, 2018

effigies left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisgorgo commented Dec 17, 2018

franklin-feingold commented Dec 17, 2018

chrisgorgo commented Dec 17, 2018