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

STYLE double-quote cython strings #21 #50013

Merged
merged 2 commits into from
Dec 2, 2022

Conversation

DeaMariaLeon
Copy link
Member

task here is:

add id: double-quote-cython-strings to

- id: cython-lint

run pre-commit run double-quote-cython-strings --all-files

git add -u, git commit -m 'double quote cython strings', git push origin HEAD`

Motivation for this comes from #49866 (comment)

pyData 2022 sprint
noatamir/pyladies-berlin-sprints#21

@jorisvandenbossche, @MarcoGorelli, @WillAyd, @rhshadrach, @phofl @noatamir

@phofl
Copy link
Member

phofl commented Dec 2, 2022

Cool! Could you merge main? There seems to be a conflict

@DeaMariaLeon
Copy link
Member Author

Cool! Could you merge main? There seems to be a conflict

Locally? or here? I got lost

@phofl
Copy link
Member

phofl commented Dec 2, 2022

Locally, and then you can push to this branch

@MarcoGorelli
Copy link
Member

you'll do it locally, and then push - something like

git fetch upstream
git merge upstream/main
<fix up merge conflicts>
git add -u
git commit
git push origin HEAD

@phofl phofl added Code Style Code style, linting, code_checks Sprints Sprint Pull Requests labels Dec 2, 2022
@DeaMariaLeon
Copy link
Member Author

DeaMariaLeon commented Dec 2, 2022

git fetch upstream
git merge upstream/main
fixed conflicts
git add -u
git commit

Run the previous commands. After that there is a test that failed

double-quote Cython strings.............................................................................Failed
- hook id: double-quote-cython-strings
- exit code: 1
- files were modified by this hook

I still tried to push to my origin (maybe I shouldn't have) but then there is a message "Everything up-to-date"

@MarcoGorelli
Copy link
Member

after that, you'll need to stage and commit again:

git add -u
git commit -a -m 'your commit message goes here'

@phofl
Copy link
Member

phofl commented Dec 2, 2022

Yep this means that the code you pulled still had single quotes. pre-commit fixed it, so if you commit again you should be good

@DeaMariaLeon
Copy link
Member Author

after that, you'll need to stage and commit again:

git add -u
git commit -a -m 'your commit message goes here'

after fixing merge conflicts, git add and commit has to be done twice?

@phofl
Copy link
Member

phofl commented Dec 2, 2022

Only if pre commit fails

@MarcoGorelli
Copy link
Member

pre-commit will block your commit if any of the hooks' checks fail, or if they modified your files. here, one of them modified your files. so you'll need to stage (git add -u) and commit (git commit -m 'commit message') again

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@phofl phofl added this to the 2.0 milestone Dec 2, 2022
@DeaMariaLeon
Copy link
Member Author

Should I just wait now? :)

@phofl
Copy link
Member

phofl commented Dec 2, 2022

Yep the ci runs around 2 hours. We will merge after everything is green

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Dec 2, 2022

thanks @DeaMariaLeon ! in before more merge conflicts

@MarcoGorelli MarcoGorelli merged commit 353d955 into pandas-dev:main Dec 2, 2022
@DeaMariaLeon
Copy link
Member Author

Thanks All for your patience :)

@DeaMariaLeon DeaMariaLeon deleted the pre-commit-config branch March 30, 2023 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks Sprints Sprint Pull Requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants