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.

Sign up for GitHub

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

Jump to bottom

Update pre-commit to check modified files only #14784

Closed
Jackie6 opened this issue Apr 3, 2019 · 1 comment · Fixed by #14971
Closed

Update pre-commit to check modified files only #14784

Jackie6 opened this issue Apr 3, 2019 · 1 comment · Fixed by #14971
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement.

Comments

@Jackie6
Copy link
Contributor

Jackie6 commented Apr 3, 2019

Is your feature request related to a problem? Please describe.
If you modify some files and want to push them, you should:

  1. git add modified files, call the files VersionA
  2. git commit -m "some messages", then the program will run npm run -s precommit to check lint styles and some other things.
  3. You encounter some errors like "Multiple spaces found", so you change the code again and save, call the files VersionB. Then git commit -m "some messages", you succeed.
  4. git push

But you find the code you push is VersionA, this is because you do not git add before you commit again in step 3. Is it possible to modify the precommit script such that it only checks the files that are modified and added to the staging area?

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
@gziolo gziolo added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Enhancement A suggestion for improvement. Needs Dev Ready for, and needs developer efforts labels Apr 10, 2019
@Jackie6
Copy link
Contributor Author

Jackie6 commented Apr 13, 2019 •
edited
Loading

There is a more detailed description of similar issue. I think it is a bug of lint-staged rather than Gutenberg. And there are many discussions about the potential solutions to this problem. Finally, the author of lint-staged announced the support for partially staged files. If we upgrade the version of lint-staged to 8.1.5, this bug will disappear.

@Jackie6 Jackie6 mentioned this issue Apr 13, 2019
Merged
5 tasks
ntwb pushed a commit that referenced this issue Apr 13, 2019
@Jackie6 @ntwb
Update the version of lint-staged (#14971)
3aca80f 
## Description
Fix #14784 

## How has this been tested?
1. `git add modified-files`, call the files VersionA
2. `git commit -m "some messages"`, then the program will run `npm run -s precommit` to check lint styles and some other things.
3. Encounter some errors like "Multiple spaces found", so you change the code again and save, call the files VersionB. But do not `git add` again.
4. `git commit` again, failed. Because now the `lint-staged` only check staged changes (i.e, VersionA).
You should `git add` again and then `git commit`

## Screenshots <!-- if applicable -->
The screenshot of running `git commit` with old version of `lint-staged`
![image](https://user-images.githubusercontent.com/24849874/56072453-06486a80-5d65-11e9-990a-9447b14abd9a.png)
The screenshot of running `git commit` with latest version of `lint-staged`. The blue boxes highlight the difference
![image](https://user-images.githubusercontent.com/24849874/56072493-84a50c80-5d65-11e9-8373-77353c8601ac.png)


## Types of changes
Upgrade lint-staged

## Checklist:
- [X] My code is tested.
- [X] My code follows the WordPress code style. <!-- Check code: `npm run lint`, Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/ -->
- [X] My code follows the accessibility standards. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/accessibility-coding-standards/ -->
- [X] My code has proper inline documentation. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/javascript/ -->
- [X] I've included developer documentation if appropriate. <!-- Handbook: https://wordpress.org/gutenberg/handbook/designers-developers/ -->
mchowning pushed a commit to mchowning/gutenberg that referenced this issue Apr 15, 2019
@Jackie6 @mchowning
Update the version of lint-staged (WordPress#14971)
d21acbb 
## Description
Fix WordPress#14784 

## How has this been tested?
1. `git add modified-files`, call the files VersionA
2. `git commit -m "some messages"`, then the program will run `npm run -s precommit` to check lint styles and some other things.
3. Encounter some errors like "Multiple spaces found", so you change the code again and save, call the files VersionB. But do not `git add` again.
4. `git commit` again, failed. Because now the `lint-staged` only check staged changes (i.e, VersionA).
You should `git add` again and then `git commit`

## Screenshots <!-- if applicable -->
The screenshot of running `git commit` with old version of `lint-staged`
![image](https://user-images.githubusercontent.com/24849874/56072453-06486a80-5d65-11e9-990a-9447b14abd9a.png)
The screenshot of running `git commit` with latest version of `lint-staged`. The blue boxes highlight the difference
![image](https://user-images.githubusercontent.com/24849874/56072493-84a50c80-5d65-11e9-8373-77353c8601ac.png)


## Types of changes
Upgrade lint-staged

## Checklist:
- [X] My code is tested.
- [X] My code follows the WordPress code style. <!-- Check code: `npm run lint`, Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/ -->
- [X] My code follows the accessibility standards. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/coding-standards/accessibility-coding-standards/ -->
- [X] My code has proper inline documentation. <!-- Guidelines: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/javascript/ -->
- [X] I've included developer documentation if appropriate. <!-- Handbook: https://wordpress.org/gutenberg/handbook/designers-developers/ -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update the version of lint-staged Jackie6/gutenberg
2 participants
@gziolo @Jackie6