[Pulp-dev] Commit message validation

[David Davis]

Typically, when we do a z-release (or patch release), we cherrypick the
individual changes based on the redmine issues set to be released (based on
the Plaform Release field). In your example, if there was a new z-release
of crane, issues 3 and 4 would be ignored since there is no issue they are
associated with. Optimally, I would have probably created issues for
commits 3 and 4 and have attached them each to those.

For features, (which get released with feature or x.y releases), attaching
commits to issues is used for bookkeeping since these releases are branched
off master (or with pulp 2, 2-master). In your example, you could have
commit 3 have “ref #3857” and commit 4 have "closes #3857”. This way, the
third commit wouldn’t affect the state of the issue but the fourth commit
would close out the issue and both commits would be associated with #3857.

David


On Tue, Sep 11, 2018 at 4:33 PM Simon Baatz <gmbnomis at gmail.com> wrote:

> Hi David,
>
> On Tue, Sep 11, 2018 at 10:15:22AM -0400, David Davis wrote:
> >    I want to announce that commit messages will be validated for PRs
> >    against pulp’s master branch per [0]. This validation will check two
> >    things.
> >    First, the validation script checks that there’s an attached issue in
> >    the commit message. This is to prevent commits not being properly
> >    associated with the issue they've fixed. If you absolutely must commit
> >    code without an issue, add “#noissue” to the commit message.
>
> I must admit that I am still confused which changes need an issue and
> what the link is good for during the build process.  Perhaps a more
> complex example could help: We had a "mixed bag" PR for Crane a
> couple of weeks ago [0] which consisted of the following commits
> (after improving it and splitting it up into 4 commits).
>
> 4. Implement option to serve local content:
>
>   The core implementation of the feature
>
> 3. Add "repository" to stored repo data from redirect files
>
>   Preparation for the actual feature.
>
> 2. Fix app_util.validate_and_transform_repo_name()
>
>   Fix for a function to improve handling of edge cases.
>
> 1. Fix logger name for v2 views
>
>   One line "drive by fix" to fix logging in a module.
>
>
> Let's assume 1. and 2. are commits one would consider to pick for a
> stable patch release (AFAIK this is currently not happening for Crane, but
> let's pretend).  3. and 4. belong to the implementation of a new
> feature (with 4. being the "meat").  The fixes were not planned
> before, they just made sense to do during implementation/review.
>
> In this PR, only 4. had an annotation linking it to the story ("closes
> #3857").
> What would be the proper way to handle this PR with respect to issues
> and annotations in your opinion?
>
>
> [0] https://github.com/pulp/crane/pull/93/commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180911/8993beaf/attachment.htm>