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

ci(sort_cspell_json): use pre-commit #256

Merged
merged 20 commits into from
Apr 5, 2022
Merged

ci(sort_cspell_json): use pre-commit #256

merged 20 commits into from
Apr 5, 2022

Conversation

KeisukeShima
Copy link
Collaborator

Changed to use pre-commit.ci so that git push can be used in the forks.
TODO: After this PR is merged, create a tag v0.1.0.

Tested on #253

Signed-off-by: Keisuke Shima 19993104+KeisukeShima@users.noreply.github.com

@KeisukeShima
ci(sort_cspell_json): use pre-commit
3cf684b 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima KeisukeShima self-assigned this Mar 24, 2022
kenji-miyake
kenji-miyake reviewed Mar 24, 2022
View reviewed changes
Copy link
Contributor

@kenji-miyake kenji-miyake left a comment

Choose a reason for hiding this comment

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

👍

.github/workflows/pytest.yaml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
pre_commit_hooks/sort_cspell_json.py Outdated Show resolved Hide resolved
pre_commit_hooks/sort_cspell_json.py Outdated Show resolved Hide resolved
tests/test_sort_cspell_json.py Outdated Show resolved Hide resolved
pre_commit_hooks/sort_cspell_json.py Outdated Show resolved Hide resolved
pre_commit_hooks/sort_cspell_json.py Outdated Show resolved Hide resolved
pre_commit_hooks/sort_cspell_json.py Outdated Show resolved Hide resolved
pre_commit_hooks/sort_cspell_json.py Outdated Show resolved Hide resolved
pre_commit_hooks/sort_cspell_json.py Outdated Show resolved Hide resolved
@KeisukeShima
remove package.json
fbcd66f 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
Copy link
Collaborator Author

.cspell.json includes unknown words?? 😕

https://github.com/tier4/autoware-spell-check-dict/runs/5675471711?check_suite_focus=true

Warning: Unknown word (planing)
tests/test_sort_cspell_json/.cspell.answer.json:162:6 Unknown word (planing)
Warning: Unknown word (planing)
tests/test_sort_cspell_json/.cspell.input.json:162:6 Unknown word (planing)

@kenji-miyake
Copy link
Contributor

@KeisukeShima It's an exception (to detect misspelling of planning). For .cspell.json, it's ignored.

autoware-spell-check-dict/.cspell.json

Line 126 in e4be3ec

"**/*.cspell.json",

I guess you can remove the part from the input/answer.

KeisukeShima and others added 7 commits March 25, 2022 10:03
@KeisukeShima @kenji-miyake
Apply suggestions from code review
29c0d22 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>

Co-authored-by: Kenji Miyake <31987104+kenji-miyake@users.noreply.github.com>
@KeisukeShima
add type hint
2ab9b60 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
rename
6157ca3 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
refactoring
884ad11 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
refactoring
571e26d 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
remove NamedTemporaryFile
fb652dd 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
remove exception word
f4f22b0 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
Copy link
Collaborator Author

@KeisukeShima It's an exception (to detect misspelling of planning). For .cspell.json, it's ignored.

autoware-spell-check-dict/.cspell.json

Line 126 in e4be3ec

"**/*.cspell.json",

I guess you can remove the part from the input/answer.

Okay, removed f4f22b0

@KeisukeShima KeisukeShima mentioned this pull request Mar 25, 2022
Merged
@KeisukeShima
fix spell
aa0b02e 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
remove tmp_path
3e360e1 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
rename to format_cspell_json
a391926 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima
change to use input_file directory
d05fd58 
Signed-off-by: Keisuke Shima <19993104+KeisukeShima@users.noreply.github.com>
@KeisukeShima KeisukeShima mentioned this pull request Mar 25, 2022
Merged
apply suggestions
d6be47b 
Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
@kenji-miyake
Copy link
Contributor

@KeisukeShima Could you check my changes, please? 🙏

@KeisukeShima
Copy link
Collaborator Author

@kenji-miyake Oh, I am the author of this PR, so I cannot approve it. Can you please merge them?

@kenji-miyake
Copy link
Contributor

Yes, but pre-commit.ci is broken, I'll check it.

@kenji-miyake
Copy link
Contributor

pre-commit.ci run

@KeisukeShima
Copy link
Collaborator Author

KeisukeShima commented Apr 5, 2022 •
edited
Loading

@kenji-miyake
We need to update tag after this pr merged.

TODO: After this PR is merged, create a tag v0.1.0.

@kenji-miyake
Copy link
Contributor

Yeah, but I've replaced it with repo: local.

@kenji-miyake
Copy link
Contributor

Oh, I understand, it's in your fork and I've pushed to the upstream.
Will re-push my change.

Kenji Miyake added 2 commits April 5, 2022 12:36
rename directory
4eb95fb 
Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
use local hook
541317d 
Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
@kenji-miyake
Copy link
Contributor

@KeisukeShima Could you confirm two new commits?

add flagWords
bd085cf 
Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
@kenji-miyake kenji-miyake force-pushed the feature/change-sort-cspell-json branch from 24c2673 to bd085cf Compare April 5, 2022 03:39
@KeisukeShima
Copy link
Collaborator Author

@KeisukeShima Could you confirm two new commits?

Looks good to me 👍

kenji-miyake
kenji-miyake previously approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@kenji-miyake kenji-miyake left a comment

Choose a reason for hiding this comment

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

@KeisukeShima Please merge if you think the changes are okay. 🙏

@kenji-miyake
Copy link
Contributor

kenji-miyake commented Apr 5, 2022 •
edited
Loading

Ah, but pytest fails.

E       KeyError: 'flagWords'

format_cspell_json/format_cspell_json.py:46: KeyError

@KeisukeShima
Copy link
Collaborator Author

Ah, but pytest fails

Maybe the flagWords key is missing in tests/test_format_cspell_json/.cspell.input.json.

@kenji-miyake
Copy link
Contributor

Yes, I'll fix it.

fix pytest
8b32f10 
Signed-off-by: Kenji Miyake <kenji.miyake@tier4.jp>
@kenji-miyake kenji-miyake force-pushed the feature/change-sort-cspell-json branch from 1dd72da to 8b32f10 Compare April 5, 2022 03:50
@KeisukeShima KeisukeShima merged commit e9da487 into tier4:main Apr 5, 2022
@KeisukeShima KeisukeShima deleted the feature/change-sort-cspell-json branch April 5, 2022 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kenji-miyake kenji-miyake kenji-miyake approved these changes

Assignees

@kenji-miyake kenji-miyake

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KeisukeShima @kenji-miyake