conventional-commits / conventionalcommits.org

The conventional commits specification
https://conventionalcommits.org
MIT License
7.02k stars 545 forks source link

What about work in progress commits #38

Closed akaguny closed 6 years ago

akaguny commented 6 years ago

examle: i'm work on feature and dont know my code is correct or no, but i must go home. i will push a commit becouse i don't whant to miss my work. But i now that i will work on this code another time. Maybe i will need more then one ore two commits while create finish commit with end fix or feature or another one, now i create chore commit, but maybe add some wip type of the commit or another point what told me about not completed commit?

chore(client.api): try to catch error with timeout

If you whant fetch....

WIP: any description
damianopetrungaro commented 6 years ago

WIP commits does not have any sense to be written in a proper way imho, they will be squashed after your feature branch will be finished.

mekwall commented 4 years ago

@damianopetrungaro But what if you don't work with feature branches? I mostly do trunk-based development and commit directly to the master branch.

glenwinters commented 4 years ago

@mekwall In trunk-based development, each commit is expected to not break the build. If you're in the middle of your work and need to save where you're at quickly, pushing to a branch is a good solution. If your commit doesn't break the build but you're not ready to release the change, you could hide it behind a feature flag.

togakangaroo commented 4 years ago

I don't see how feature flags are an answer here, it seems like an unrelated point. Whether you use them or not, does not give you an answer as to what commit type to tag things with

glenwinters commented 4 years ago

@togakangaroo He asked for a solution when committing directly to the master branch. It doesn't make sense to indicate WIP in a commit on the master branch because it should be working and that commit is permanent. However, if you want to commit something but hide it from users until you finish some other things, then feature flags are a solution to that.

mekwall commented 4 years ago

@glenwinters A work-in-progress commit doesn't necessarily (and with trunk-based development it shouldn't) break the build. Just because something is being worked on doesn't mean it is broken, it's just not done as per definition of done. I definitely agree with you about hiding it behind a feature flag though, but that doesn't really answer my question.

There are also the scenario of where work-in-progress commits might exist is in pull/merge requests that will not be squashed when merged. For example, the Kubernetes team have the following guidelines:

General squashing guidelines:

Sausage => squash

When there are several commits to fix bugs in the original commit(s), address reviewer feedback, etc. Really we only want to see the end state and commit message for the whole PR.

Layers => don't squash

When there are independent changes layered upon each other to achieve a single goal. For instance, writing a code munger could be one commit, applying it could be another, and adding a precommit check could be a third. One could argue they should be separate PRs, but there's really no way to test/review the munger without seeing it applied, and there needs to be a precommit check to ensure the munged output doesn't immediately get out of date.

Source: https://github.com/kubernetes/kubernetes/blob/release-1.5/docs/devel/faster_reviews.md#7-fix-feedback-in-a-new-commit

akaguny commented 4 years ago

WIP commits does not have any sense to be written in a proper way imho, they will be squashed after your feature branch will be finished.

Yes, but if i want to integrate the commit hook with conventional checks my commit hook will be broken the commit.

stevemao commented 4 years ago

I think you should bypass the commit hook in that case

rikhoffbauer commented 4 years ago

To me it would be valuable to have some sort of standard way of describing a WIP commit on feature branches, though I agree WIP commits on the master branch don't make sense.

Though I am not sure if this is out of scope for Conventional Commits.

An arbitrary scenario I can imagine is working on an issue together with a colleague (pair programming) and at the end of the day wanting to commit (and push) your changes.

This allows either developer to continue work on the issue on their own if one of them is unavailable to continue working on it for any reason.

Some (to me) obvious advantages I see:

An example implementation could be adding a WIP footer, like the OP suggested e.g.:

feat: a very awesome and complex feature

WIP: implemented for POSIX, win32 implementation is in progress

This is similar to the BREAKING CHANGE footer (see Commit message with description and breaking change footer).

damianopetrungaro commented 4 years ago

Again @rikhoffbauer a WIP commit it is just a "ghost" of a commit that is going to be merged.

What I do personally do is create a WIP pull request, and push the commit without any "WIP". Once I've done all the changes needed, I remove the WIP from the PR title.

Let me try to act like the bad cop: What is the objective benefit of this feature?

rikhoffbauer commented 4 years ago

@damianopetrungaro

To me the objective benefit is being able to communicate clearly what the effects/impact of every commit is using its message at any point in time.

I believe having a standard vocabulary for indicating a commit contains code that isn't finished yet provides developers that read the git log with valuable information.

To me it doesn't matter that the WIP commit (and its message) won't be in the git history in the end because of squashing, merging, amending or whatever. At some point it is this commit is in the git history and its message should in my opinion should still communicate as clearly as possible what the impact of the commit is.

I think the WIP keyword in commit messages should only be used as a last resort or incidentally, e.g. when a task isn't divisible into smaller subtasks is only implemented in part or isn't finished in some other way but is committed (and pushed) nonetheless so that other people can continue working on it, or in order to persist it somewhere other than on your local machine.

WIP commits should be avoided but can be useful/valuable from time to time in my opinion.

An arbitrary scenario where a WIP commit might be valuable is when pair programming a complex (part of a) feature that isn't divisible into smaller subtasks and "saving" (committing and pushing) the progress so that it is clear to your coworkers that is an intermediary commit but they still have access to the code and continue working on it or view it.

Consider this analogy with a video game. In this analogy:

Some levels are very difficult which is why the game offers a "save game" feature (WIP commit) you can access at any point in time to save your current progress (a WIP commit).

If this is out of scope or you don't think this is valuable or desirable that's fine and we can agree to disagree. I just hope I was able to clearly communicate what I mean and why I feel its valuable.

damianopetrungaro commented 4 years ago

Why not having a WIP merge request then?

It sound way more reasonable to me and Gitlab uses already this format as well.

You can define a WIP merge request and have always clean commits.

damianopetrungaro commented 4 years ago

And also you may split your work in different commits that you are gonna change later on (rebasing them and fix them up). You may have a section already part of the specification to split the different chenges affecting one component. Example:

fix(email): remove nil pointers fix(customer): remove nil pointers

Or if you're not working in different sections but in a single one you can still specify what you changed, example:

fix: remove nil pointers on the customer object fix: remove nil pointers on the email sender

And all those are part of a PR

WIP: fix: remove nil pointers

vviotto-masti commented 4 years ago

One of the reasons to use a commit convention is to automatically bump versions and generate a change log in CI/CD.

Lets say for example that we have feature A, B and C already on the master branch (assuming a trunk based development). Now lets say that management requested a new release for today, for whatever reason (It could be a simple case of competition, releasing first etc). Everything is ok, except, a critical bug has been found in feature B, but we still want to release feature A and C. In that case, we could hide feature B behind a feature flag and release the new version while we fix the critical bug, when the bug is fixed we would probably have 2 more commits, one for the bugfix and one to switch the feature flag so feature B can be used.

This is something that can happen, and in that case if we commit features A, B and C with a "feat" commit message, all 3 features would appear in the release change log. Now the question is, assuming all WIP features are behind a feature flag until validated (a QA environment could have these flags turned on for example), how would we tag commits for:

1- The feature commit with a flag turned off 2- The commit that turns the flag on and officially "release" the feature for the public.

I think that this scenario is in line with the OP question, the specification should describe a way to deal with WIP commits (this is the same as features behind feature flags).

samuelpetroline commented 4 years ago

The debate on whether you should ignore or not, commit or not depends on the context of each development team. If you think it makes sense for you, here is a quick solution (note that you need commitlint):

https://stackoverflow.com/questions/60194822/how-do-you-configure-commitlint-to-ignore-certain-commit-messages-such-as-any-th

ndabAP commented 4 years ago

I came across this as an open source maintainer and agree with @rikhoffbauer. Time shouldn't be a factor for technical documentation: Why is the duration between the WIP commit and the squashed state not relevant?

Imagine writing feat: add login but you only added the files and it's a WIP. What a miss communication for all other developers. I'm not looking into all commits I see. We are usually working in a collective. I trust the commit message and for me feat: add login signals a finished task. Especially in open source programming, where people usually don't meet and less organization is involved, clear signals are important. The WIP flag could be used to automatically highlight a WIP commit, e. g.

What I do personally do is create a WIP pull request

@damianopetrungaro, a pull request is for me out of context. They exist in a world of Git repository SaaS but Conventional Commits exclusively belongs to Git. The request was to add a way to communicate within Git and not a SaaS.

jheaff1 commented 3 years ago

Say you are developing a feature where the user presses a button on the GUI which invokes a call to a TCP server. I would personally structure my commits as:

  1. Implement TCP server
  2. feat: Add fancy button to GUI

What type should the first commit be? It is not a user-facing feature (feat:), nor a bug fix, or a chore, or a work-in-progress. It adds a TCP server to the repository that could later be reused by the implementation of future features.

damianopetrungaro commented 3 years ago

Sorry to get back to you after so long, got really busy in the last few months sadly!

I got what all of you are saying, trust me, but my main objective is that "WIP commits" make sense only when you are still working on something which is not finished.

@rikhoffbauer @ndabAP @vviotto-masti (tagging you since you were quite strongly opinionated on this) keep in mind what is the purpose of conventional commits and most importantly that each team can add a layer on top of that.

You may, for example:

Adding a "wip" nomenclature in the actual convention will make things less clear and more complex to distinguish when or when not to use a "wip" or even when or not to merge those commits, which is IMHO a pretty big price to pay for a tiny benefit.

damianopetrungaro commented 3 years ago

@jheaff1 I'd personally do sth like:

   feat: Add fancy feature
   This feature adds XYX buttons to enable the customer to do amazing things!
   Refs: ticket-numebr

The code needed for that is both server/client sides, and the commit message provides the "big picture" of the feature. If you then use multiple repositories for both server(s) and client(s) the message can be still as well!

The way I like to make sure my commit is good is by answering this question:

what if I rollback this commit? will the whole feat/fix/whatever be entirely removed without leaving some of its code into the source?

quantumwebco commented 2 years ago

What's wrong with doing feat: WIP the task you've been working on is feat: only for completed features?

mekwall commented 2 years ago

What's wrong with doing feat: WIP the task you've been working on is feat: only for completed features?

Not ideal if you use tools to generate changelogs based on commit messages. You probably don't want your changelog to list features that are WIP.

WygorFonseca commented 2 years ago

Why not create a WIP prefix. Eg.:

WIP: the task you've been working on

togakangaroo commented 2 years ago

On the contrary. If you do that then it makes it trivial to filter out any wip commits either with a rebase, a log filter, or with a grep of the changelog.

It is not easy if there is no such standard.

On Sun, Aug 21, 2022, 14:57 Marcus Ekwall @.***> wrote:

What's wrong with doing feat: WIP the task you've been working on is feat: only for completed features?

Not ideal if you use tools to generate changelogs based on commit messages. You probably don't want a changelog littered with WIP commits.

— Reply to this email directly, view it on GitHub https://github.com/conventional-commits/conventionalcommits.org/issues/38#issuecomment-1221610855, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQEZQJ5DTACXQR2E4LONTV2KC3TANCNFSM4EWTZFAA . You are receiving this because you were mentioned.Message ID: @.*** com>

Michaelvsk commented 1 year ago

@jheaff1 I'd personally do sth like:

   feat: Add fancy feature
   This feature adds XYX buttons to enable the customer to do amazing things!
   Refs: ticket-numebr

The code needed for that is both server/client sides, and the commit message provides the "big picture" of the feature. If you then use multiple repositories for both server(s) and client(s) the message can be still as well!

Doesn't that mean, that a single commit could become quite large? If I understand you correctly, if you are not relying on PRs with squashing, then any new feature must be a single commit?

Isn't that contrary to committing early and often?

How would you handle commits to share code with other devs on the same feature branch? E.g. You have implemented the backend and then a frontend-dev is going to implement the UI consuming that functionality to fully implement the actual feature.

I am especially asking about how to handle it without relying on squashing as I did work on a project where squashing was disabled.

SaadBazaz commented 9 months ago

In my opinion, wip makes perfect sense. When compiling a list of feature notes, once can choose to ignore all wips, or to use them as "Description of approach" / "Methodology".

I stumbled upon this issue when trying to look for alternatives.

feat or fix doesn't cut it, imo.

MasonFI commented 8 months ago

Shame thas

@jheaff1 I'd personally do sth like:

   feat: Add fancy feature
   This feature adds XYX buttons to enable the customer to do amazing things!
   Refs: ticket-numebr

The code needed for that is both server/client sides, and the commit message provides the "big picture" of the feature. If you then use multiple repositories for both server(s) and client(s) the message can be still as well!

Doesn't that mean, that a single commit could become quite large? If I understand you correctly, if you are not relying on PRs with squashing, then any new feature must be a single commit?

Isn't that contrary to committing early and often?

How would you handle commits to share code with other devs on the same feature branch? E.g. You have implemented the backend and then a frontend-dev is going to implement the UI consuming that functionality to fully implement the actual feature.

I am especially asking about how to handle it without relying on squashing as I did work on a project where squashing was disabled.

Shame that @damianopetrungaro hasn't answered this.

My proposal is that you use WIP-commits like wip(#task): description

That should leave fairly good history trail for specific feature, because #task is either a feature or a child task of a feature.

And in case you need to remove all implementations of specific feature, or see its history, with this commit history you can fetch all related commits easily.

Then you also can exclude wip-commits from release notes or changelog.

How does this sound like?

sbrow commented 5 months ago

I think that Conventional Commits should have an official opinion on WIP commits, or at least have some references to some common solutions to the problem.

Story Time

I have been using Conventional Commits for at least 5 years now- I like how it forces you to slow down, break your work into distinct chunks, and not just throw a bunch of unrelated changes into one commit. This system works. However, when the only work that can be committed is completed work, it means your incomplete work is left uncommitted.

This has always worked for me, leaving my incomplete work sitting uncommitted on top of HEAD.

Two weeks ago, I was working on a large refactor, and although I had a feature branch for it, it didn't make sense to refactor just one small system at a time- it was less work to refactor them all at once (or so I thought). Since committing unfinished work would break the build, I left my work uncommitted for three days...

...and then I hecked up. I accidentally selected my entire project directory when attempting to delete a single, empty ( lol ) file in my IDE (neovim). To be clear, I do not blame Conventional Commits for my idiocy, but I do think that CC can do more to prevent others from making the same mistake.

I did a root cause analysis on what happened, and made the following decisions:

  1. I should configure neo-tree to delete to Trash instead of rming things.
  2. Work should always be committed, at the end of the day at least.

Solutions

I like the convention of marking work-in-progress commits with wip(optional scope): Description, because it's simple, idiomatic, and it works. However, the way I learned to use git was to use merges, and to treat the git history as immutable (or close to it). For this reason, I was afraid of accidentily committing WIP commit, and then committing something else on top of it. So, I wrote a git hook.

1. pre-commit-msg Hook

With this solution, you create a work-in-progress commit with a message matching wip(scope): description. You push this commit to your feature branch, and as you continue working on it, you amend the commit, and then push the amendment. This hook prevents you from committing on top of a WIP commit, so you (and everyone else) know it's not safe to work there.

#!/usr/bin/env nu

# Run as a prepare-commit-msg hook. Prevents you from creating a new commit
# on top of a "wip" commit.
def main [
  commit_msg_file: path
  commit_type: string
  sha?: string
] {
    if (wip? $sha) and (not (amending? $sha)) {
      print -e (
      $"(ansi red)Cannot commit on top of a Work-In-Progress commit(ansi reset)"
      )

      exit 1
    } else {
      exit 0
    }
}

# Returns true if the previous commit message starts with "wip"
def "wip?" [ sha?: string ] {
  if ($sha | is-empty) {
    git log -1 --pretty=%B
  } else {
    git log -1 --pretty=%B $sha
  } | str starts-with -i wip
}

# Retuns ture if the active commit is amending the previous commit
def "amending?" [commit_sha?: string] {
  ($commit_sha | is-empty | not $in) or (staged-files | is-empty)
}

# Returns a list of the files that are staged for commit
def staged-files [] {
  git diff-index --cached --name-only HEAD | lines
}

This is by no means foolproof, nor is it perfect, but it worked well enough for me at first.

2. Rebasing

Shortly after coming up with solution 1, I started learning about alternate git strategies like Stacked PRs and stacked commits. I've since decided to use more flexible schemes that allow you to change git history, by using rebasing instead of merges. I started with the excellent git-branchless, until, through their own documentation, I was led to jujutsu. Now, I no longer worry about accidentally committing on top of a WIP commit, because I know I can re-arrange them at will.

Conclusion

Maybe I'm a terrible programmer, but I think acting like every commit needs to be a single, contained change that is absolutely perfect the first time is living in a fantasy land. The longer you spend working locally on your feature commit, you more frustrated you will be when you accidentally delete something, or your hard drive dies, or you lose power.

Decide on a way to mark work-in-progress commits, so that everyone on your team knows they're not safe to commit on top of. Commit early and often, regardless of your commit's completeness.

Don't screw up like I did.

Like I said, I accept responsibility for what happened, but I also think it is a fault in the Conventional Commits system to not handle the concept of works-in-progress.

Some things are too big to leave sitting around, but can't reasonably be broken up into multiple "features".

That's my 2 cents. Maybe I'm just a nutjob, but the 30 other :-1:s I see at the top of this thread seem to indicate that other people struggle with this problem as well.

fterrani commented 3 months ago

I think this must be mentioned on the official specification. It can be explained/nuanced of course, but I think we need to at least suggest something, even if it's in the FAQ section. My take on this is that it's useful since as a developer:

I like the wip type idea because:

But I think it is crucial to specify what is in progress exactly though. It can be a feature but it can certainly be many other things!

I would either enclose the original type and scope in wip (drawback: not compatible with current syntax?):

wip(feat(csv-export)): Added CSV export for half of the columns
wip(build): Added spell check for french strings (german left to do)
wip(docs(foo)): Documenting feature foo (it takes time...)

Or use a <type>-<scope> string that will be used as the wip scope (which is backward compatible?):

wip(feat-csv-export): Added CSV export for half of the columns
wip(build): Added spell check for french strings (german left to do)
wip(docs-foo): Documenting feature foo (it takes time...)

My preference goes for the <type>-<scope> syntax, both for readability and backward compatibility.

What do you think?

If enough people see this message and deem this idea useful, I think it's more than time to create a pull request suggesting this change to get things moving. A FAQ recommendation wouldn't be normative ; people would still be free to deviate from it and at least it will provide some guidance to those interested.