Skip to content

Comments

tuf/api: Expose tuf.api as a package (take 2)#1177

Merged
lukpueh merged 9 commits intotheupdateframework:developfrom
joshuagl:ww/tuf-api-package
Oct 15, 2020
Merged

tuf/api: Expose tuf.api as a package (take 2)#1177
lukpueh merged 9 commits intotheupdateframework:developfrom
joshuagl:ww/tuf-api-package

Conversation

@joshuagl
Copy link
Member

Taking over from #1157

Description of the changes being introduced by the pull request:

  • add tuf/api/init.py, turning tuf/api into a full package for distribution purposes
  • add a pylintrc for tuf/api and update tox to use it (and ignore tuf/api with the default pylintrc)
  • minor stylistic changes to tuf/api/metadata.py to resolve some pylint warnings

Note: ac8d273 disables a single pylintrc check in a code comment. I was loath to do this, but I think it's better to explicitly disable that site than disable the message for all code before we've decided on coding style.

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@joshuagl joshuagl requested a review from lukpueh October 15, 2020 10:56
@joshuagl joshuagl changed the title tuf/api: Expose tuf.api as a package tuf/api: Expose tuf.api as a package (take 2) Oct 15, 2020
Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @joshuagl! Looks mostly good to me.

woodruffw and others added 8 commits October 15, 2020 14:35
Signed-off-by: William Woodruff <william@trailofbits.com>
Add a minimal pylintrc to lint for new code being developed in tuf/api and
update the tox configuration to ignore tuf/api with the default pylintrc
and run an extra invocation of pylint for just the modules in tuf/api.

Signed-off-by: Joshua Lock <jlock@vmware.com>
The logging module is not used in metadata, therefore remove it

Signed-off-by: Joshua Lock <jlock@vmware.com>
A single letter variable name of 'f' causes pylint to throw a coding style
convention warning:

C0103: Variable name "f" doesn't conform to snake_case naming style
(invalid-name)

Signed-off-by: Joshua Lock <jlock@vmware.com>
Using an else after a raise results in a refactor message from pylint:

R1720: Unnecessary "elif" after "raise" (no-else-raise)

This is because the raise will exit the block, and pylint suggests that
explicit if's, rather than an if-elif-else, are clearer style. Update the
style of Metadata.verify() to match pylint expectations.

Signed-off-by: Joshua Lock <jlock@vmware.com>
The Targets constructor takes seven arguments, which violates pylints
default value of five for max-arguments:

R0913: Too many arguments (7/5) (too-many-arguments)

As this feels like a coding style decision that should be made and
documented disable that test for only the Targets constructor until
a coding style decision has been made and documented as a decision
record.

Signed-off-by: Joshua Lock <jlock@vmware.com>
We don't need to lint the code with every version of Python, instead add
an extra tox env which lints once with the latest supported Python version

Signed-off-by: Joshua Lock <jlock@vmware.com>
Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl
Copy link
Member Author

I've force pushed an update to squash fixups, which prevents the DCO from complaining.

You can see the pre rebase history here develop...joshuagl:ww/tuf-api-package-hist where I've added 81e0ef6 to add the lint env to travis, and move bandit into the lint env. I also changed 427759b while I was in the .travis.yml to use newer Python for with-sslib-master

@woodruffw
Copy link
Contributor

Thanks @joshuagl!

Authored-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Signed-off-by: Joshua Lock <jlock@vmware.com>
Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

🎉

@lukpueh lukpueh merged commit a64a334 into theupdateframework:develop Oct 15, 2020
@joshuagl joshuagl deleted the ww/tuf-api-package branch October 15, 2020 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants