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

CI cannot run on PRs from forks because repo variables are not shared with forks #79

Closed
2 of 3 tasks
surchs opened this issue Mar 9, 2024 · 1 comment · Fixed by #80
Closed
2 of 3 tasks

CI cannot run on PRs from forks because repo variables are not shared with forks #79

surchs opened this issue Mar 9, 2024 · 1 comment · Fixed by #80
Assignees
Labels
quick fix Minimal planning and/or implementation work required. released This issue/pull request has been released.

Comments

@surchs
Copy link
Contributor

surchs commented Mar 9, 2024

For example #63. This is annoying because it's now extra work to figure out of the tests are failing for a good reason or just because of the fact that the variables are not shared. Even worse, dependabot counts as a fork too, and based on this discussion: https://github.com/orgs/community/discussions/44322 we can expect that dependabot runs will have the same issue.

Right now, we only use repo variables in two workflows:

- name: Create .env file
run: |
echo -e "NB_API_QUERY_URL=${{ vars.NB_API_QUERY_URL }}\nNB_IS_FEDERATION_API=${{ vars.NB_IS_FEDERATION_API }}" > .env

and

- name: Create .env file
run: |
echo -e "NB_API_QUERY_URL=${{ vars.NB_API_QUERY_URL }}\nNB_IS_FEDERATION_API=${{ vars.NB_IS_FEDERATION_API }}" > .env

and there is only one variable that could realistically change (the API URL). I suggest we

  • get rid of the repo variables
  • update the e2e workflow to hardcode the API URL
  • update the component test workflow to also hardcode the API URL
@surchs surchs added this to Neurobagel Mar 9, 2024
@surchs surchs added maint:coverage quick fix Minimal planning and/or implementation work required. labels Mar 9, 2024
@surchs surchs moved this to Implement - Active in Neurobagel Mar 9, 2024
@surchs surchs self-assigned this Mar 9, 2024
@surchs surchs moved this from Implement - Active to Implement - Done in Neurobagel Mar 9, 2024
@rmanaem rmanaem moved this from Implement - Done to Review - Active in Neurobagel Mar 12, 2024
@github-project-automation github-project-automation bot moved this from Review - Active to Review - Done in Neurobagel Mar 13, 2024
@surchs
Copy link
Contributor Author

surchs commented Apr 11, 2024

🚀 Issue was released in v0.2.0 🚀

@surchs surchs added the released This issue/pull request has been released. label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quick fix Minimal planning and/or implementation work required. released This issue/pull request has been released.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant