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

more elegant use of GitHub-set env variables #500

Closed
cgreene opened this issue Jul 24, 2023 · 10 comments · Fixed by #503
Closed

more elegant use of GitHub-set env variables #500

cgreene opened this issue Jul 24, 2023 · 10 comments · Fixed by #503

Comments

@cgreene
Copy link
Contributor

cgreene commented Jul 24, 2023

I attempted to set BUILD_DOCX to true by adjusting the GitHub environment variables for the project. This didn't appear to be used. Adding an env section and pulling in that variable appeared to work per this PR: https://github.com/greenelab/optimizer-manuscript/pull/21/files

Is this something that would be desired for one or more of the build configuration variables? I assume a PR would include adding the env options + noting the variables setup this way in the documentation. I'm not aware of an elegant way to test this, though.

@agitter
Copy link
Member

agitter commented Jul 24, 2023

That looks like a good way to do it. I believe that is similar to what the second paragraph of the build readme is describing: https://github.com/manubot/rootstock/tree/main/build.

However, the typical Manubot user does not know to navigate to the build instructions to see documentation about environment variables and how to enable DOCX output. We could link to these instructions from the usage, add more support for environment variables, or do something else.

@cgreene
Copy link
Contributor Author

cgreene commented Jul 24, 2023

I do think that's what the second paragraph describes, but I didn't find that the BUILD_DOCX variable was actually used without adding the env section to manubot. Perhaps there was a change in behavior?

@agitter
Copy link
Member

agitter commented Jul 25, 2023

The env section does need to be added to the workflow file, which is what the linked GitHub doc is trying to describe. If that is unclear, we should rewrite the rootstock build instructions paragraph.

I tested that in a demo manuscript, and it generated a docx output gitter-lab/bds-srop-demo-manuscript@5d54318

Where you trying to control the environment variables for the project somewhere else outside of the workflow file?

@agitter
Copy link
Member

agitter commented Jul 25, 2023

Got it. I'd say we need a docs improvement to

  • point to the build instructions from usage so this is easier to find in the first place
  • better explain how to set the env variable in the workflow and distinguish between that and the configuration variables in a repository

@cgreene
Copy link
Contributor Author

cgreene commented Jul 25, 2023

Do you want a PR that would use those variables if they are set but not have an impact otherwise, or should we leave that as an exercise for the user?

@agitter
Copy link
Member

agitter commented Jul 25, 2023

I'm fairly indifferent. My main preference is that there is one primary way of changing environment variables when doing things with Manubot instead of splitting it between modifying workflow yaml files and repo settings. We are somewhat oriented toward modifying yaml files based on the AI revision workflow variables and spellcheck variable in the main workflow.

Let's wait for @dhimmel's thoughts too before making a PR.

@dhimmel
Copy link
Member

dhimmel commented Jul 25, 2023

Okay so I think @cgreene added BUILD_DOCX as a repository action variable in the settings at https://github.com/<USER>/<REPO>/settings/variables/actions. Was it the following GitHub menu you used at /settings/variables/actions/new?

image

It looks like we'd have to access those in the workflow using the vars context rather than the env context (env context is made available to the shell and therefore would trigger BUILD_DOCX properly).

@cgreene are you aware of the one time builds using workflow dispatch (as shown below)? Do you want a DOCX just for some builds that can be manually triggered or all builds?

image

I have a slight preference for keeping persistent configuration in code rather than in repo settings (secrets being the obvious exception). If this feature generates more interest we could reconsider?

@cgreene
Copy link
Contributor Author

cgreene commented Jul 26, 2023

I did add them that way. I am aware of that feature but have experienced challenges in the past when folks didn't generate word docs at each stage & journals wanted word comparisons to show review changes. I guess the key would be to add a standard operating procedure to tag anything submitted to a journal - then one could re-run with the tag to get the word files for word's compare documents feature.

@dhimmel
Copy link
Member

dhimmel commented Aug 3, 2023

I guess the key would be to add a standard operating procedure to tag anything submitted to a journal - then one could re-run with the tag to get the word files for word's compare documents feature.

Great idea. It's never too late to create a tag and the tag would enable dispatching the workflow for the past version of the manuscript.

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 a pull request may close this issue.

3 participants