-
Notifications
You must be signed in to change notification settings - Fork 19
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
Refactor: import-targets
to import-artifacts
#421
Refactor: import-targets
to import-artifacts
#421
Conversation
4a9a1c8
to
4cf72c5
Compare
- Updates the user commands to show as `import-artifacts` instead `import-targets` following the changes in the API and documentation. Signed-off-by: Kairo de Araujo <[email protected]>
4cf72c5
to
4d92647
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #421 +/- ##
=======================================
Coverage 98.64% 98.64%
=======================================
Files 16 16
Lines 1181 1181
=======================================
Hits 1165 1165
Misses 16 16
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few recommendations where to rename targets
to artifacts:
- https://github.com/kairoaraujo/repository-service-tuf-cli/blob/4d92647e388a496fda5cce45abc5cf707850b200/pyproject.toml#L49-L50
- https://github.com/kairoaraujo/repository-service-tuf-cli/blob/4d92647e388a496fda5cce45abc5cf707850b200/repository_service_tuf/cli/admin/ceremony.py#L216
- !!! BUG !!! https://github.com/kairoaraujo/repository-service-tuf-cli/blob/4d92647e388a496fda5cce45abc5cf707850b200/repository_service_tuf/helpers/api_client.py#L23 I am in favor of renaming the enum URL from
public_targets
topublic_artifacts
here - If you rename
publish_targets
above you can consider doing here:publish_targets: bool = True
@@ -711,13 +712,14 @@ sign (``sign``) | |||
|
|||
|
|||
.. rstuf-cli-admin-import-targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. rstuf-cli-admin-import-targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is to not break the docs in the umbrella until we update the sphinx import links.
╰──────────────────────────────────────────────────────────────────────────────╯ | ||
╭─ Commands ───────────────────────────────────────────────────────────────────╮ | ||
│ ceremony Start a new Metadata Ceremony. │ | ||
│ import-artifacts Import artifacts to RSTUF from exported CSV file.│ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
│ import-artifacts Import artifacts to RSTUF from exported CSV file.│ | |
│ import-artifacts Import artifacts to RSTUF from exported CSV file. │ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do that you need to add a space to the other rows as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can keep it. It is just an example to the user in the docs., unless there are other changes to do.
Signed-off-by: Kairo de Araujo <[email protected]>
Done Done
I'm not renaming now. We can do it as another issue. This PR was intended to change the user views. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Added an issue for the rest of my comments: #422
Description
import-artifacts
insteadimport-targets
following the changes in the API and documentation.Types of changes
Additional requirements
Code of Conduct
By submitting this PR, you agree to follow our Code of Conduct.