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

Changelogs content, formatting and links #53

Closed
janpio opened this issue Nov 21, 2018 · 3 comments
Closed

Changelogs content, formatting and links #53

janpio opened this issue Nov 21, 2018 · 3 comments
Labels
enhancement New feature or request

Comments

@janpio
Copy link
Member

janpio commented Nov 21, 2018

During apache/cordova-docs#911 by @brodybits I noticed that our previous rules and guidelines for changelogs don't work any more:

apache/cordova-docs#911 (comment):

Some of the changelog items link to PRs, some to items.

When JIRA was a thing, they all linked to the issue they fixed. Now we don't always have issues, and it is not really defined how to handle this. In my opinion it makes sense to link all to the PR, as the PR usually includes a link to the issue it closes. Agree? Or issue and PR both in a specific way?

A proper "Squash and merge" commit message for example includes the PR number in the commit message: cordova-android 7.1.3 blog post (#911) (for this PR). Maybe we should use this template, link the #911 to the PR and add an issue in front if available?

@brodybits replied:

apache/cordova-docs#911 (comment)

In my opinion it makes sense to link all to the PR, as the PR usually includes a link to the issue it closes.

I'm fine to do it that way for now, would like to see feedback from others.

A proper "Squash and merge" commit message for example includes the PR number in the commit message: cordova-android 7.1.3 blog post (#911) (for this PR). Maybe we should use this template, link the #911 to the PR and add an issue in front if available?

  1. departure from convention of using CB-??? or GH-??? that I saw elsewhere, looks like this came from Apache convention

  2. just using #??? is not so good since we have a toolset with quite a few different repos. I would favor something like apache/cordova-android#550 to refer to PR #550 on cordova-android, like how GItHub references issues & PRs.

  3. moving the reference from the beginning to the end is yet another departure

We can continue the discussion here now.

@janpio
Copy link
Member Author

janpio commented Nov 21, 2018

Interesting points @brodybits

  1. is good point, we will have to investigate if this is a real convention or just a thing we copied from somewhere and should be kept.
    The GH-xxx was actually just a crutch after we moved from JIRA to GitHub and didn't want to change the convention to much I think.

  2. is indeed true. It is what GitHub does by default in the commit message when merging a PR via the "Squash + Merge" button, but it totally makes sense that we add the repository before that so it becomes explicit. (We should probably include that in some tooling that automates this)

  3. is also caused by what GitHub does in the commit messages, also with JIRA we only linked the issue - not the PR - in the changelog which is why it might sense to continue to do this (prefix by issue ID if exists, suffix with PR ID always). But maybe we can also not link to the issue at all as all PRs should include a link to an issue (or be the issue at the same time) as well?

Other input?

@brodycj brodycj changed the title Changelogs Changelogs with PR numbers Nov 21, 2018
@brodycj
Copy link

brodycj commented Nov 21, 2018

(1) departure from convention [...]

[...]
The GH-xxx was actually just a crutch [...]

Agreed, and I find it to be an ugly one. I am really coming to favor to just use "#???" like GitHub does.

(2) just using #??? [...]

[...] It is what GitHub does by default in the commit message when merging a PR via the "Squash + Merge" button, but it totally makes sense that we add the repository before that so it becomes explicit.

I had only meant that a more explicit reference should be in the release notes in cordova-docs. But I said in apache/cordova-docs#911 (comment) that the more explicit reference may not be so important if a release notes page is all for the same package.

(3) moving the reference from the beginning to the end

is also caused by what GitHub does in the commit messages

Yes.

Some of my early experience with GitHub merges came from watching the changes on PouchDB, and they seem to do some manual work to put the original PR number at the beginning of every commit before landing in the repo. A recent example is pouchdb/pouchdb#7534, which they landed in pouchdb/pouchdb@405e8d3. (Earlier PouchDB PRs such as pouchdb/pouchdb#2900 were actually closed rather than merged since the GitHub merge tool was less advanced back then. And Node.js still seems to close accepted PRs with "landed" messages instead of actually merging them; nodejs/node#23163 is an example.)

I am starting to favor keeping the PR number at the end, like GitHub does by default, to make things easier. Committer should feel free to edit the merge commit message or not, as appropriate per PR.

But maybe we can also not link to the issue at all as all PRs should include a link to an issue (or be the issue at the same time) as well?

I would favor that approach now. Not all PRs originate from separate issues, and some PRs may originate from issues in https://github.com/apache/cordova/issues (this project).

@janpio I just took the liberty to update the title to better reflect the discussion here.

@janpio janpio changed the title Changelogs with PR numbers Changelogs content, formatting and links Nov 22, 2018
@janpio
Copy link
Member Author

janpio commented Nov 22, 2018

I like how this turned out in https://cordova.apache.org/announcements/2018/11/21/cordova-android-7.1.3.html 👍 Clean and simple.

@janpio janpio added the enhancement New feature or request label Apr 10, 2019
@janpio janpio closed this as completed Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants