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

Replace usage of UnityBegin/UnityEnd with UNITY_BEGIN/UNITY_END #947

Merged

Conversation

ahans
Copy link
Contributor

@ahans ahans commented Jan 28, 2024

See discussion here

Done via:

for f in $(find . -name "test_*.c"); do
    sed -i -e 's/UnityBegin\(.*\);/UNITY_BEGIN();/' $f
    sed -i -e 's/UnityEnd/UNITY_END/' $f
done

@ahans ahans force-pushed the feature/use-unity_begin-without-filename branch 3 times, most recently from d078939 to 92b8f62 Compare January 28, 2024 22:30
See discussion [here](exercism#943 (review))

Done via:
```
for f in $(find . -name "test_*.c"); do
    sed -i '' 's/UnityBegin\(.*\);/UNITY_BEGIN();/' $f
    sed -i '' 's/UnityEnd/UNITY_END/' $f
done
```
@ahans ahans force-pushed the feature/use-unity_begin-without-filename branch from 92b8f62 to 99d271c Compare January 28, 2024 22:38
Copy link
Member

@ryanplusplus ryanplusplus left a comment

Choose a reason for hiding this comment

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

Thanks!

@ryanplusplus ryanplusplus merged commit baf90ef into exercism:main Jan 29, 2024
4 checks passed
@siebenschlaefer
Copy link
Contributor

I know it's too late now, but for future changes like this please add [no important files changed] to the commit message of the merge. Otherwise thousands of solutions get re-compiled and re-tested which can take a long time and be expensive.
see https://exercism.org/docs/building/tracks#h-avoiding-triggering-unnecessary-test-runs

@ahans ahans deleted the feature/use-unity_begin-without-filename branch January 29, 2024 11:08
@ahans
Copy link
Contributor Author

ahans commented Jan 29, 2024

I know it's too late now, but for future changes like this please add [no important files changed] to the commit message of the merge. Otherwise thousands of solutions get re-compiled and re-tested which can take a long time and be expensive. see https://exercism.org/docs/building/tracks#h-avoiding-triggering-unnecessary-test-runs

Oh, I had no idea! Sorry about this!

Would it be a good idea to provide a PR template that contains a reminder about this, as in

  • `[no important files changed] has been added to commits that don't affect tests

@ryanplusplus, what do you think?

@ryanplusplus
Copy link
Member

@ahans There's supposed to be an automated comment from an exercism bot to notify/remind about this. I'm not sure why it didn't fire for your PR, though...

@ryanplusplus
Copy link
Member

@ahans There's supposed to be an automated comment from an exercism bot to notify/remind about this. I'm not sure why it didn't fire for your PR, though...

See #949 (comment) for an example.

@wolf99
Copy link
Contributor

wolf99 commented Jan 29, 2024

Could be good to update the track documentation for this as I think we may reference this somewhere there.

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.

4 participants