-
Notifications
You must be signed in to change notification settings - Fork 92
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
sync GH actions with IUC #582
sync GH actions with IUC #582
Conversation
.github/workflows/pr.yaml
Outdated
@@ -203,7 +238,7 @@ jobs: | |||
- name: lintr | |||
run: | | |||
library(lintr) | |||
linters <- with_defaults(line_length_linter = NULL, cyclocomp_linter = NULL) | |||
linters <- with_defaults(line_length_linter = NULL, object_usage_linter = NULL) |
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.
Should we keep cyclocomp_linter = NULL
(added in commit 4e17a8d )?
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.
Probably yes and we should also add it to the IUC workflow, or?
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 suppose it would make life easier on tools-iuc too.
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.
done. and galaxyproject/tools-iuc#3398
- name: Install Planemo | ||
run: pip install planemo | ||
- name: Deploy on testtoolshed | ||
env: | ||
SHED_KEY: ${{ secrets.tts_api_key }} | ||
SHED_KEY: ${{ secrets.TTS_API_KEY }} |
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.
@nsoranzo do you know if the change to uppercase requires an update of the settings?
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, it does need someone with admin privileges on this repo to remove and re-add the TTS and TS API key secrets.
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.
OK. Given the activity on this repo we can ask for this change after this is merged.
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 doubt this is needed, we have uppercase version in the secrets since June
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 remember that they are shown upper case even if entered lower case. But we can just try with one of my long open PRs :)
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.
@bernt-matthias lol, in that case I have no idea what we have. I am ready to change it if it fails though.
458d94c
to
c332641
Compare
Ready to go? |
Seems that deployment works https://github.com/galaxyproject/tools-devteam/actions/runs/451752521 |
seems that the PR workflow does not work anymore, because of
set-env
. Guess its about time to sync the workflows again.