-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix action to work with new version of the CLI + add more tests #5
Conversation
- uses: actions/setup-python@v4 | ||
with: | ||
python-version: "3.10" | ||
- name: deps | ||
run: pip install ruff | ||
- uses: actions/checkout@v3 | ||
- name: lint | ||
run: ruff upload.py |
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.
My formatter (prettier) did this formatting.
If you want to keep it in the old style, I can revert this
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.
no preference
if the computer is happy I'm happy
elif args.name: | ||
meta_args = ('--name', args.name) | ||
|
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.
This didn't seem to be populated before
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 didn't test the no-meta case; makes sense
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.
for clarity -- you're intentionally removing the org-id
arg from the update
command?
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.
Yes, the org-id and namespace arguments don't make much sense for the update command because the update command requires a meta.json (which includes the module_id (which includes namespace and orgid)).
Back when name
existed, it kind of vaguely made sense, but it really was just complication because it is possible to write it that way, not because it was actually good behavior.
- uses: actions/setup-python@v4 | ||
with: | ||
python-version: "3.10" | ||
- name: deps | ||
run: pip install ruff | ||
- uses: actions/checkout@v3 | ||
- name: lint | ||
run: ruff upload.py |
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.
no preference
if the computer is happy I'm happy
elif args.name: | ||
meta_args = ('--name', args.name) | ||
|
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 didn't test the no-meta case; makes sense
elif args.name: | ||
meta_args = ('--name', args.name) | ||
|
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.
for clarity -- you're intentionally removing the org-id
arg from the update
command?
See test run here: #4