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

[Documentation] Commit message guidelines for cleaner commit history #2418

Closed
IshitaTakeshi opened this issue Jan 19, 2022 · 9 comments
Closed
Assignees
Labels
priority:low Lower urgency, can be addressed later.

Comments

@IshitaTakeshi
Copy link

The current commit history is automatically generated when merging, but it creates many lines and makes it harder to catch up on what is going on. So I'd like to propose formulating a commit message guideline for better commit history.

I think we should explicitly define what we should include in the commit history and what we don't include. We can refer to other OSS contribution guidelines.

@kenji-miyake
Copy link
Contributor

  • When do you confirm the commit history?
  • What command do you?

Please give me a bit more concrete examples. 🙏

@IshitaTakeshi
Copy link
Author

IshitaTakeshi commented Jan 19, 2022

When exploring in which commit the changes have been made to interested parts, if the commit message has many lines it makes us harder to find what we are looking for.
Let's say someone made changes to the localization module, modified the NDT's optimization function, and I want to improve the functionality.
If commit message was like

Improve NDT
* tmp 
* update
* remove a for loop
* ...

I can not obtain any information from there.
If every commit message was like this I cannot even find in which commit the changes I want to refer to have been made.

@kenji-miyake
Copy link
Contributor

@IshitaTakeshi Again, what command do you use? And please paste it with the output like this.
Also, why do you not use --oneline?

❯ git log --oneline
920b874 (HEAD -> tier4/main, origin/tier4/proposal, origin/tier4/main, origin/HEAD, tier4/proposal) fix(dummy_perception): fix ns and topic (#242)
6f111e0 fix(behavior_path_planner): fix/frenet coordinate 3d (#230)
699b60e feat: use point cloud wraper in ring outlier filter (#780) (#241)
5f09a5d feat(simple_planning_simulator): add delay model of velocity and steering (#235)
f4993f3 fix(behavior_velocity_planner): fix before/after line of virtual_traffic_light (#228)
b0e4852 fix: interpolation (#791) (#218)
aa75dac fix: library path (#225)
b4eba36 fix: set control_mode false before autoware engage (#232)
784f10d fix: remove warning for compile error (#198)
52dc0f2 ci: update .yamllint.yaml (#229)
ed3253c chore: remove unnecessary depends (#227)
1657681 fix: order to create callback (#220)

@IshitaTakeshi
Copy link
Author

I used the ordinary git log. So then I'm going to use git log --oneline

@IshitaTakeshi
Copy link
Author

This is the output. Too long to paste the whole though

tier4/proposal $git log 
commit 4cec60485bb376f9cc7d85448be782c2852f445a (HEAD -> tier4/proposal, origin/tier4/proposal, origin/HEAD)
Author: Kenji Miyake <[email protected]>
Date:   Tue Jan 18 16:17:52 2022 +0900

    feat: add ansible setup scripts (#8)
    
    * ci: add build CI
    
    Signed-off-by: Kenji Miyake <[email protected]>
    
    * feat: add files for Ansible Galaxy
    
    Signed-off-by: Kenji Miyake <[email protected]>
    
    * feat: copy ansible files from tier4's proposal
    
    Remove cache_valid_time
    
    Signed-off-by: Kenji Miyake <[email protected]>
    
    v0.4.0
    
    Signed-off-by: mitsudome-r <[email protected]>
    
    copy setup
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    add ros2 ansible roles
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    ansible add ubuntu20 setup
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    ansible fix pacmod
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    temp disable cuda & tensorrt
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    ansible fix kvaser library repo
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    temporally comment out for unused modules
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    remove unused codes
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    Install osqp only once (#95)
    
    * update readme and fix osqp_vendor tag
    
    * Remove ansible osqp role
    
    fix: ansbile role
    
    Closes #140
    
    add ansible for cuda 11.1 & tensorrt 7.2.1 & cudnn 8.0.5
    
    Signed-off-by: Takamasa Horibe <[email protected]>
    
    add script for install livox sdk
    
    Signed-off-by: kosuke murakami <[email protected]>
    

@kenji-miyake
Copy link
Contributor

@IshitaTakeshi I see, thank you!

Then, how about using these useful tools? It helps you to explore the commit history.

Although these are for VSCode, there might be tools for your IDE.

@IshitaTakeshi
Copy link
Author

Yeah but... It's like messing a library by ourselves and exploring a book from there. If the library is clean from the beginning we don't need to do so. And it can be realized by writing just two or three phrases when making a commit
Why making unnecessary jobs by messing our rooms by ourselves...

@kenji-miyake
Copy link
Contributor

@IshitaTakeshi Could you write clear acceptance criteria for this issue, please?

@kenji-miyake kenji-miyake added the priority:low Lower urgency, can be addressed later. label Feb 13, 2022
@kenji-miyake
Copy link
Contributor

I'll close this because there is no response and I think it's a problem of tool usage.

We use https://www.conventionalcommits.org/en/v1.0.0/ and Squash merge now. I'll write guideline documents about that and it's tracked in autowarefoundation/autoware-documentation#6.

@mitsudome-r mitsudome-r transferred this issue from autowarefoundation/autoware_core_universe_prototype Jun 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:low Lower urgency, can be addressed later.
Projects
None yet
Development

No branches or pull requests

2 participants