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

Problem: prefix makes git history look like a list of problems #117

Open
anarcat opened this issue Sep 23, 2016 · 9 comments
Open

Problem: prefix makes git history look like a list of problems #117

anarcat opened this issue Sep 23, 2016 · 9 comments

Comments

@anarcat
Copy link
Contributor

anarcat commented Sep 23, 2016

I question the use of Problem: in the changelog summary. I believe it makes the commit history look like a list of bug instead of a list of solutions. It may be just semantic nitpicking but I prefer to see a summary of the solution in the summary, then details of the solution in the longer version of the commitlog. It seems to be a common standard as well: Linux, for example, mandates to "Describe the technical detail of the change(s) your patch includes." as opposed to the actual problem it resolves, which can still be refered to in the longer description. (source)

I understand where this is coming from, and maybe it's a good standard for an established project that is just fixing bugs, or that is used to that convention. But it is certainly surprising when coming from a more traditionnal background.

I would like to suggest turning that MUST into a SHOULD or MAY.

I agree, however, that a patch MUST adress an existing issue or known problem. It seems to me we already have something to that effect, however, earlier on (although it is "SHOULD be a minimal and accurate answer to exactly one identified and agreed problem" - maybe that should be a MUST?)

@bluca
Copy link
Member

bluca commented Sep 23, 2016

While I see your point and where it comes from, on the other hand I think "Problem: we cannot do X" is a valid statement that describes a feature.

@anarcat
Copy link
Contributor Author

anarcat commented Sep 23, 2016

if you read the RFC, sure: otherwise it is just confusing. It looks like the commit introduces problem X, maybe as a corrolary.

when i saw the git history of this project, I didn't understand what the commitlogs meant until I read C4.

@hintjens
Copy link
Member

We've adopted this over time as a successful pattern, and codified in the
RFC. There are a lot of surprising things in C4. Casting every change as a
problem is one I'd strongly prefer to keep. The MUST might be a SHOULD,
indeed.

On 23 Sep 2016 11:48 pm, "anarcat" [email protected] wrote:

if you read the RFC, sure: otherwise it is just confusing. It looks like
the commit introduces problem X, maybe as a corrolary.

when i saw the git history of this project, I didn't understand what the
commitlogs meant until I read C4.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#117 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AASzCG311w6fsegA1cUFyqvA_HUmU6MUks5qtEkxgaJpZM4KFaoh
.

@sappo
Copy link
Member

sappo commented Sep 24, 2016

The Problem statement wields enormous power. At first you need to
recognize that every bug, every feature, every change and every idea is
essentially a problem. Thus having a list of solved problems comes
naturally. Using the Problem statement also sharpens the awareness
whether a feature, bug, change or idea is actually worth solving as it
is really easy to see if there is a need. Further it forces developers
to consciously fragment their work into a stream of small fixes rather
than one big with a rubbish commit message or even a listing.

From a maintainers perspective it allows us to quickly examine a pull
request and merge it in a manner of seconds or decline it if the
problem statement is false. And probably most important it is a
powerful tool to identify bad actors as they wont follow rules.

@hintjens
Copy link
Member

You said it better than I could... :) Absolutely accurate.

On 24 Sep 2016 6:50 pm, "Kevin Sapper" [email protected] wrote:

The Problem statement wields enormous power. At first you need to
recognize that every bug, every feature, every change and every idea is
essentially a problem. Thus having a list of solved problems comes
natural. Using the Problem statement also sharpens the awareness
whether a feature, bug, change or idea is actually worth solving as it
is really easy to see if there is a need. Further it forces developers
to consciously fragment their work into a stream of small fixes rather
than one big with a rubbish commit message or even a listing.

From a maintainers perspective it allows us to quickly examine a pull
request and merge it in a manner of seconds or decline it if the
problem statement is false. And probably most important it is a
powerful tool to identify bad actors as they wont follow rules.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#117 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AASzCNK4HTYxanrkthLjoSLEYSx8FOfXks5qtVTrgaJpZM4KFaoh
.

@anarcat
Copy link
Contributor Author

anarcat commented Sep 24, 2016 via email

@hintjens
Copy link
Member

Yes, opinion noted. We have evolved this specific style over years and used
it in many projects with success. We consider it foundational. We know it
is surprising to newcomers. So be it.

On 24 Sep 2016 10:31 pm, "anarcat" [email protected] wrote:

I absolutely agree with you about the usefulness of the "Problem"
statement, and you make a great point at explaining why it is useful.

I just don't think it's appropriate that it's the summary of the commit
(the first line of the commit message). I think it should be a mandatory
part of the message, but the summary should be just that: a summary of
why something or what was done.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#117 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AASzCIRNvClqB3ZvdeaNR6H6LdRFq1Rzks5qtYiWgaJpZM4KFaoh
.

@sappo
Copy link
Member

sappo commented Sep 25, 2016

IMO, this issue is not a problem but exactly what we intended! Though feel
free to use C4 with your modifications towards commit messages in one of
your projects to see if it works and report back your experiences :)

Am 25.09.2016 00:37 schrieb "Pieter Hintjens" [email protected]:

Yes, opinion noted. We have evolved this specific style over years and
used
it in many projects with success. We consider it foundational. We know it
is surprising to newcomers. So be it.

On 24 Sep 2016 10:31 pm, "anarcat" [email protected] wrote:

I absolutely agree with you about the usefulness of the "Problem"
statement, and you make a great point at explaining why it is useful.

I just don't think it's appropriate that it's the summary of the commit
(the first line of the commit message). I think it should be a mandatory
part of the message, but the summary should be just that: a summary of
why something or what was done.


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#117 (comment), or
mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AASzCIRNvClqB3ZvdeaNR6H6LdRFq1Rzks5qtYiWgaJpZM4KFaoh

.


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#117 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAeGupOxAh6bWLvUznxpVpUa3JuxSwv2ks5qtaZCgaJpZM4KFaoh
.

@christianbundy
Copy link

Interesting -- we started using a development process derived from C4 in Oasis and I thought that the format was:

Replace foo with bar for Y

Problem: The foo module doesn't implement interface X, which we need for Y.
Solution: Remove the foo module and replace it with bar, which implements X via Z.

I've just looked at the ZeroMQ commit messages for the first time and noticed that it's not what we've been doing. I was also confused by the "Problem:" commit messages, but now understand that we've been Doing It Wrong Differently in our implementation.

mssawant pushed a commit to mssawant/cortx-ha-1 that referenced this issue Jan 30, 2021
Solution: make "Problem:" prefix a recommendation, not a strict
requirement.  s/MUST/SHOULD/

[RFC 2119](https://tools.ietf.org/html/rfc2119):

> 3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
>    may exist valid reasons in particular circumstances to ignore a
>    particular item, but the full implications must be understood and
>    carefully weighed before choosing a different course.

See also zeromq/rfc#117

Closes Seagate#361.

[ci skip]
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

No branches or pull requests

5 participants