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

Update common to its latest version #5517

Merged
merged 1 commit into from
May 9, 2019
Merged

Update common to its latest version #5517

merged 1 commit into from
May 9, 2019

Conversation

humitos
Copy link
Member

@humitos humitos commented Mar 21, 2019

Get new configs from common.

@stsewd
Copy link
Member

stsewd commented Mar 21, 2019

@stsewd
Copy link
Member

stsewd commented Apr 13, 2019

We can remove the object inheritance (there are just a couple of places).

But we also have pylint: logging-format-interpolation / Use % formatting in logging functions and pass the % p which is in a lot of places and isn't a little to refactor :/, so is better to ignore the formatting check for now I guess :(

@humitos
Copy link
Member Author

humitos commented Apr 30, 2019

@stsewd I think making our code to pass the linter completely could be a "Good First Issue". Marking it as that to allow folk find it and helps us on this.

The actionable here is:

  1. update our common/ submodule to the latest master
  2. run tox -e lint to find out the problems
  3. adapt the code to fix those problems
  4. run tox -e lint and make it succeed --iterating between 2 and 3

@humitos humitos added Accepted Accepted issue on our roadmap Good First Issue Good for new contributors Improvement Minor improvement to code labels Apr 30, 2019
@stsewd
Copy link
Member

stsewd commented Apr 30, 2019

If anyone wants to send a PR for this, I'd appreciate separating it in little chunks, so it's easy to review :)

@stsewd stsewd mentioned this pull request Apr 30, 2019
16 tasks
@saadmk11
Copy link
Member

saadmk11 commented May 1, 2019

@stsewd I have found 50+ pylint: logging-format-interpolation / Use % formatting in logging functions and pass the % p issues. how do you want the PRs to be separated ? should I add different PRs for Different files?

@stsewd
Copy link
Member

stsewd commented May 1, 2019

I think for each constant or module.

@saadmk11
Copy link
Member

saadmk11 commented May 3, 2019

Created All the PRs needed to fix the lint issues. :)

Copy link
Member

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

Linter is fixed!

Thanks @saadmk11!

@stsewd stsewd merged commit 571f0b9 into master May 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the humitos/update-common branch May 9, 2019 15:11
@saadmk11
Copy link
Member

saadmk11 commented May 9, 2019

Glad I could Help ! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Good First Issue Good for new contributors Improvement Minor improvement to code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants