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

Apis docstirngs and improved logging, fix create repository #325

Merged
merged 10 commits into from
Jul 10, 2023

Conversation

renatav
Copy link
Collaborator

@renatav renatav commented Jul 3, 2023

Description (e.g. "Related to ...", etc.)

  • Added missing docstrings, adopting the newest format used in TUF
  • Fixed create repository command following extensive refactoring
  • Work on improving logging, see Improve logging #27

Code review checklist (for code reviewer to complete)

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Commit messages are meaningful (see this for details)
  • Tests have been included and/or updated, as appropriate
  • Docstrings have been included and/or updated, as appropriate
  • Changelog has been updated, as needed (see CHANGELOG.md)

@renatav renatav self-assigned this Jul 3, 2023
@renatav renatav requested a review from n-dusan July 3, 2023 14:55
Copy link
Contributor

@n-dusan n-dusan left a comment

Choose a reason for hiding this comment

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

Minor nits. Could we also update changelog?

taf/api/repository.py Outdated Show resolved Hide resolved
Comment on lines -423 to +579
repository = repository._tuf_repository
"""
Return role TUF object based on its name
"""
if isinstance(repository, Repository):
repository = repository._repository
Copy link
Contributor

Choose a reason for hiding this comment

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

Will repository be None if repository is not an instance of Repository?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TBH, this was a quick fix because repository was either a TAF repository or a TUF repository (which is what repository._repository returns. We should look into all calls and standardize the input (either expect one or the other). Not sure if that's worth cramming into this PR because it's mostly focused on logging and docstirng. We could create an issue to make sure we don't forget about it

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, let's create an issue

taf/api/targets.py Outdated Show resolved Hide resolved
@renatav renatav requested a review from n-dusan July 9, 2023 23:39
@n-dusan n-dusan merged commit 0338f69 into master Jul 10, 2023
@n-dusan n-dusan deleted the renatav/docstrings-and-logging branch July 10, 2023 09:38
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.

2 participants