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

Change Travis mentions to "CI" or remove as appropriate #671

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

Conversation

fulldecent
Copy link
Contributor

This removes dead code for Travis CI which has not run in years. Updates wording mentioning TravisCI to instead mention CI.

@drinckes
Copy link
Contributor

drinckes commented Jan 8, 2025

Thanks!

@drinckes drinckes self-assigned this Jan 8, 2025
@@ -22,13 +22,8 @@ RETURN=0
for FILE in `ls *.[ch] */*.[ch]`; do
DIFF=`diff $FILE <($CLANG_FORMAT $FILE)`
if [ $? -ne 0 ]; then
if [ -z "$TRAVIS" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The original intention is to format the files if running locally, but to fail the CI if there are differences.

Removing this will format the file in the CI test, but not commit it, and the CI will succeed.

I suggest replacing $TRAVIS with $GITHUB_WORKFLOW, here and below, since that should be set in the CI run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is our intention that GitHub CI will have access to make commits here and rewrite code for style? Or just that CI will error on code style failure?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just throw errors. I think I had it doing commits in the Travis CI days long ago but I'd rather keep it simple tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants