Skip to content
This repository has been archived by the owner on Aug 18, 2023. It is now read-only.

Add yapf linter step and make CI explicit #116

Merged
merged 7 commits into from
Dec 17, 2021
Merged

Add yapf linter step and make CI explicit #116

merged 7 commits into from
Dec 17, 2021

Conversation

jaeyoo
Copy link
Contributor

@jaeyoo jaeyoo commented Dec 16, 2021

Fix #44

Before this PR, CI only showed two steps only in this Conversation Tab.

Continuous Integration / Checks (pull_request) Failing after 5s 
cla/google Successful in 1s — ✅ All contributors are covered under a CLA with Google

This made us frustrated to check the real cause of the error because it's implicit. So, this PR makes the CI steps explicit in this Conversation tab, which also enables the parallel executions of the whole CI steps.

Also, it adds yapf linter

@jaeyoo jaeyoo requested review from zaqqwerty and farice December 16, 2021 22:00
@farice
Copy link
Contributor

farice commented Dec 16, 2021

omg you're so fast that I got whiplash!

would it make sense to fix whatever errors yapf raises in the same PR?

@jaeyoo
Copy link
Contributor Author

jaeyoo commented Dec 16, 2021

Since there are so many lint errors, let me open a new PR to deal with it.

@farice
Copy link
Contributor

farice commented Dec 16, 2021

sounds great, thanks for fighting the beast! looking forward to seeing the green checkmark! 💪

@zaqqwerty
Copy link
Contributor

To ease the burden, is there a way to add an excludelist that initially has all the files, then we remove them one-by-one in PRs? (new files would not be added to the list)

@jaeyoo
Copy link
Contributor Author

jaeyoo commented Dec 16, 2021

To ease the burden, is there a way to add an excludelist that initially has all the files, then we remove them one-by-one in PRs? (new files would not be added to the list)

That's a good idea. let me add the excludelist for linter, and defeat the lint errors one-by-one :)

@jaeyoo
Copy link
Contributor Author

jaeyoo commented Dec 16, 2021

After PyPI recently changed their client webauth scheme (1 hr ago), poetry can't find packages for now. Let's wait for their patch...

@jaeyoo
Copy link
Contributor Author

jaeyoo commented Dec 17, 2021

Done. may I merge this? @zaqqwerty

Copy link
Contributor

@zaqqwerty zaqqwerty left a comment

Choose a reason for hiding this comment

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

Thanks Jae, LGTM

@jaeyoo jaeyoo merged commit c9f10fe into main Dec 17, 2021
@zaqqwerty zaqqwerty mentioned this pull request Dec 20, 2021
@zaqqwerty zaqqwerty deleted the fix_ci_add_lint branch February 6, 2022 08:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove remaining lint and add check to CI
3 participants