Bids-specification: PR titles

Created on 14 Feb 2019  Â·  8Comments  Â·  Source: bids-standard/bids-specification

@bids-standard/everyone

Hi BIDS Community!

Regarding the PR titles - when opening a PR, please have the PR title provide context to what is being merged. These titles will appear in our changelog automatically (when the PR has been merged).

Thank you!

Most helpful comment

Mainatining a readable changelog with different levels of abstraction is challenging without adding extra manual labor. However, we can improve on this step by step - I suggest adding this to the contributing guidelines and pull request template:


Naming convention for pull request:

  • Please keep the name of your pull request short but informative - it will appear in the changelog
  • If you are opening a pull request to obtain early feedback, but the changes are not ready to be merged (a.k.a. Work in Progress pull request) please use draft pull request
  • Use one of the following prefixes in the title of your pull request:

    • [ENH] - enhancement of the specification that adds a new feature or support for a new data type

    • [FIX] - fix of a typo or language clarification

    • [INFRA] - changes to the infrastructure automating the specification release (for example building HTML docs etc.)

    • [MISC] - everything else including changes to the file listing contributors


This would be a good short term solution, but in the future I think it would be better to replace title tags with github labels. This would, however, require changes to the infrastructure generating changelogs that would need to read the labels and render them in the changelog.

I also opened another issue for allowing to modify (correct) the changelog post merge. https://github.com/bids-standard/bids-specification/issues/164

All 8 comments

As examples, it would probably be good to look at pregh-changes.md to get a sense of the level of description that's useful.

It would also be good to have some kind of tag, like [minor] or [nospec] that we can put in titles for things that don't affect the semantics of the spec, like modifying the code that performs the rendering or continuous integration. (That said, we might still want those PRs to be credited in the release blurbs.)

How about tags like [MRG] in this regard? Perhaps it would be nicer to have this removed in the future --> especially with the new github feature of internally tagging a PR as WIP or MRG without having to convey it in the title: https://github.blog/2019-02-14-introducing-draft-pull-requests/

Yes, I personally would want the titles at merge time to be exactly what we want in the changelog, so tags like [MRG] and [WIP] are definitely no good.

Though we may also be able to automatically filter those out. If we can filter, perhaps a better approach would be to have [SPEC] in entries we actively want in the spec changelog. If that's the case, dropping [*] would be an easy addition, so any tags could continue to be used.

Yeah, I think I (and other novices) are going to need some more guidance
about this. Can we add a "Naming your pull request" section to the
CONTRIBUTING.md file? In this section can we have:

  1. a table of tags that are recommended and what they mean (I still
    haven't figured out what [MRG] stands for..)
  2. Some examples of commit messages and how they will render to the
    change log?

On Fri, Feb 15, 2019 at 8:35 AM Chris Markiewicz notifications@github.com
wrote:

Yes, I personally would want the titles at merge time to be exactly what
we want in the changelog, so tags like [MRG] and [WIP] are definitely no
good.

Though we may also be able to automatically filter those out. If we can
filter, perhaps a better approach would be to have [SPEC] in entries we
actively want in the spec changelog. If that's the case, dropping [*]
would be an easy addition, so any tags could continue to be used.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/bids-standard/bids-specification/issues/150#issuecomment-464052623,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMl59jTDAOYNPtabohkpqOJaJS0247BQks5vNretgaJpZM4a8hbq
.

Yes, we should definitely have docs for describing these and giving advice, once we settle on the process.

I'm not sure what all bracket tags will make sense with a document repository instead of a software repository, but [WIP] and [MRG] are "Work in Progress" and "Merge" as in "Ready-to-merge". These conventions seem to float freely between initialisms and severe contractions.

As an aside, I expect much of the burden of this to come down on people accepting the merge, editing the title to make a good changelog entry if necessary. But that should still definitely come with docs, and I think it would be helpful for contributors to think about a concise statement of the semantic change they're proposing, as this will also be what people scrolling through the open PRs and issues will see.

Totally agree. I didn't realize it was possible for the person accepting
the merge to change the title. That definitely makes things easier.

For changes to the main spec, it 's nice of the message to index the
section that has changed (i.e. 4.2 task..) that need double brackets in the
raw commit message?

On Fri, Feb 15, 2019 at 10:37 AM Chris Markiewicz notifications@github.com
wrote:

Yes, we should definitely have docs for describing these and giving
advice, once we settle on the process.

I'm not sure what all bracket tags will make sense with a document
repository instead of a software repository, but [WIP] and [MRG] are
"Work in Progress" and "Merge" as in "Ready-to-merge". These conventions
seem to float freely between initialisms and severe contractions.

As an aside, I expect much of the burden of this to come down on people
accepting the merge, editing the title to make a good changelog entry if
necessary. But that should still definitely come with docs, and I think it
would be helpful for contributors to think about a concise statement of the
semantic change they're proposing, as this will also be what people
scrolling through the open PRs and issues will see.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/bids-standard/bids-specification/issues/150#issuecomment-464093657,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMl59hMsP9NbUyoUL-ivb9q3un2sKOP6ks5vNtQ7gaJpZM4a8hbq
.

I just want to +100 @effigies' suggestion above about a [SPEC] tag. I had brought up the challenge with the automated change list in #133. You can see that @franklin-feingold answers the question https://github.com/bids-standard/bids-specification/issues/133#issuecomment-457301167 but it's really hard to see the answer in src/CHANGES.md.

I think some guidelines in CONTRIBUTING.md makes sense.

I think at the moment the plan is for PRs to automatically merge after 5 days (#147) if there are two approving pull requests (DECISION-MAKING.md). I think this means reviewers should be checking for appropriate change messages before they approve the PR.

Mainatining a readable changelog with different levels of abstraction is challenging without adding extra manual labor. However, we can improve on this step by step - I suggest adding this to the contributing guidelines and pull request template:


Naming convention for pull request:

  • Please keep the name of your pull request short but informative - it will appear in the changelog
  • If you are opening a pull request to obtain early feedback, but the changes are not ready to be merged (a.k.a. Work in Progress pull request) please use draft pull request
  • Use one of the following prefixes in the title of your pull request:

    • [ENH] - enhancement of the specification that adds a new feature or support for a new data type

    • [FIX] - fix of a typo or language clarification

    • [INFRA] - changes to the infrastructure automating the specification release (for example building HTML docs etc.)

    • [MISC] - everything else including changes to the file listing contributors


This would be a good short term solution, but in the future I think it would be better to replace title tags with github labels. This would, however, require changes to the infrastructure generating changelogs that would need to read the labels and render them in the changelog.

I also opened another issue for allowing to modify (correct) the changelog post merge. https://github.com/bids-standard/bids-specification/issues/164

Was this page helpful?
0 / 5 - 0 ratings

Related issues

chrisgorgo picture chrisgorgo  Â·  10Comments

Lestropie picture Lestropie  Â·  10Comments

sappelhoff picture sappelhoff  Â·  9Comments

tsalo picture tsalo  Â·  7Comments

greydongilmore picture greydongilmore  Â·  10Comments