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

Add CI for wikize refs #1937

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add CI for wikize refs #1937

wants to merge 8 commits into from

Conversation

markcmiller86
Copy link
Member

Resolves #729

This is a workflow to run wikize-refs.py on any .md file in the PR not in docs subfolder.

Running wikize-refs.py on a non-wikized file is harmless and will return 0 status.
Running wikize-refs.py on a wikized file will prodoce non-zero status if it would change the file.

@markcmiller86
Copy link
Member Author

You can see examples of its run on a pass and failure here, https://github.com/markcmiller86/test-wikize-refs-ci/pull/8/commits

@bartlettroscoe
Copy link
Member

@markcmiller86, I have been having some issues with wikize_refs.py with recent articles. The problem occurs when I try to rerun wikize_refs.py in-place it reports that references are missing (or something like that). I can reproduce those and post a new issue for those. I think until we get to the bottom of that, I don't think we should set up a GHA to run this on the repo.

@bartlettroscoe
Copy link
Member

@markcmiller86, the error shown here shows:

+ for file in $files
+ utils/wikize_refs.py -u README.md
Error: Process completed with exit code 1.

That output does not seem to show the actual STDOUT/STDERR from wikize_refs.py from the line:

utils/wikize_refs.py -u "$file"

and it does not print the error from the line:

echo "$file needs wiki refs updated"

It almost seems like the statement utils/wikize_refs.py -u README.md exists the tool right away. That is not the desired behavior. We want to show the errors in all of the files, not just the first file with an error.

Can we create a bash script that that is run by this *.yml file that we can test and debug locally and then just have the *.yml file call that script? For example, see Build workflows as independent steps that can be tested locally and Mock Github Environment Variables for local testing in Best Patterns: Building Github Actions workflows.

@markcmiller86
Copy link
Member Author

@markcmiller86, the error shown here shows:

+ for file in $files
+ utils/wikize_refs.py -u README.md
Error: Process completed with exit code 1.

That output does not seem to show the actual STDOUT/STDERR from wikize_refs.py from the line:

Yes, you are correct. It looks like it is default behavior of GitHub workflow to exit upon first shell command returning non-zero exit status. So, I can adjust that using either set +e at the beginning of the shell script or continue-on-error: true to the yaml step.

utils/wikize_refs.py -u "$file"

and it does not print the error from the line:

echo "$file needs wiki refs updated"

It almost seems like the statement utils/wikize_refs.py -u README.md exists the tool right away. That is not the desired behavior. We want to show the errors in all of the files, not just the first file with an error.

Yes, see above.

Can we create a bash script that that is run by this *.yml file that we can test and debug locally and then just have the *.yml file call that script? For example, see Build workflows as independent steps that can be tested locally and Mock Github Environment Variables for local testing in Best Patterns: Building Github Actions workflows.

No, we don't need to do that. It is in fact working.

It is not an error to run wikize_refs.py and for it to discover the new file it will generate is different from the input. In fact, that is typically how it is run. But, with -u (--up-to-date) command-line option to wikize_refs.py, it becomes an error. It just returns a non-zero status and is otherwise silent. I should make it silent when it would return a zero status in that case too.

Honestly, I think its working and doing exactly what it needs to other than the set +e above which I will commit a change for.

As to your other issue about you having problems...I don't think its really fair to hold up this PR due to un-reported problems the tool apparently has. Please submit a new PR for those. But, lets not hold up this PR for those. I can't honestly know to fix them if they are unknown to me.

@markcmiller86
Copy link
Member Author

@bartlettroscoe raised an unreported issue in a preceding comment suggesting we hold off on this. Nonetheless, I would like to get this finished and committed. And, it has been working for me in my testing of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

Add automation to "validate" wikized references in articles
3 participants