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 stsewd commented Mar 21, 2019

@stsewd
Copy link
Member

@stsewd 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 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

@stsewd
Copy link
Member

@stsewd 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 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 stsewd commented May 1, 2019

I think for each constant or module.

@saadmk11
Copy link
Member

@saadmk11 saadmk11 commented May 3, 2019

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

@stsewd stsewd force-pushed the humitos/update-common branch from 42c4591 to 1f304a2 May 9, 2019
@stsewd stsewd force-pushed the humitos/update-common branch from 1f304a2 to 75b4ef1 May 9, 2019
stsewd
stsewd approved these changes May 9, 2019
Copy link
Member

@stsewd stsewd left a comment

Linter is fixed!

Thanks @saadmk11!

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

@saadmk11 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants