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-cd: add env variables needed to generate binder links in online doc #89

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

nhuet
Copy link
Contributor

@nhuet nhuet commented Oct 27, 2021

The build.yml and release.yml are modified to define the environment variables needed by autodoc.py to generate the github and binder links for notebooks after PR #85.

  • variables for binder env are "hard-coded" to point to airbus/scikit-decide:binder
  • variables for notebooks content are defined thanks to github variables available in actions

The resulting doc with proper github/binder links can be checked

Copy link
Contributor

@dbarbier dbarbier left a comment

Choose a reason for hiding this comment

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

In order to play nicely with #75, please do not touche these steps, but instead create a new step which sets these variables in GITHUB_ENV.

If build.yml is run when pushing a tag, this is a mistake, we only want to run release.yml, so you can remove the handling of this case.

@nhuet
Copy link
Contributor Author

nhuet commented Oct 28, 2021

If build.yml is run when pushing a tag, this is a mistake, we only want to run release.yml, so you can remove the handling of this case.

This is the case at least on my fork. The "on push" event triggers when pushing commits and tags. Perhaps we should restrict this (in another PR :) )

@nhuet
Copy link
Contributor Author

nhuet commented Oct 28, 2021

Build.yml is also triggered in airbus repo for tags, see: https://github.com/airbus/scikit-decide/actions?query=event%3Apush+branch%3Av0.9.2
I will do a PR to prevent this.

@nhuet nhuet force-pushed the nh/add-env-variables-for-binder-links branch from 29ff287 to 6652e57 Compare October 28, 2021 10:25
@nhuet
Copy link
Contributor Author

nhuet commented Oct 28, 2021

New step created as suggested by #89 (review)

@dbarbier
Copy link
Contributor

Thanks, but please make this new step the first one of build-docs (in both files), to make sure there will be no conflict with Generate. This step only modifies GITHUB_ENV, thus having it as a first step looks fine, even if these environment variables are only used at the end.

- name: Set env variables for github+binder links in doc
run: |
# binder environment repo and branch
AUTODOC_BINDER_ENV_GH_REPO_NAME="airbus/scikit-decide"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why should we want to use the airbus/scikit-decide binder on a fork and not the local binder ?

Copy link
Contributor

@dbarbier dbarbier Oct 28, 2021

Choose a reason for hiding this comment

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

To make sure that everything works as expected when pull request is merged.

@nhuet nhuet force-pushed the nh/add-env-variables-for-binder-links branch from 6652e57 to 11dfe71 Compare October 28, 2021 15:55
- variables for binder env "hard-coded" to point to airbus/scikit-decide:binder
- variables for notebooks content defined thanks to github variables
  available in actions
- done as first step to avoid conflicts with other PRs
@nhuet nhuet force-pushed the nh/add-env-variables-for-binder-links branch from 11dfe71 to e94fe6b Compare November 2, 2021 15:10
@galleon galleon merged commit 39961b1 into airbus:master Nov 10, 2021
@nhuet nhuet deleted the nh/add-env-variables-for-binder-links branch November 26, 2021 08:58
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.

3 participants