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

Run clang_format_check in deterministic location #768

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

Conversation

wxmerkt
Copy link
Contributor

@wxmerkt wxmerkt commented Jan 6, 2022

This is a proposal to implement #767. It enables to use the AFTER_INSTALL_CLANG_FORMAT_CHECK hook to download a clang-format file into /tmp/clang_format_check/$PROJECT_NAME

@wxmerkt wxmerkt force-pushed the topic/clang-format-check-in-deterministic-location branch from 7f7c70b to 0acf08f Compare January 6, 2022 22:23
Copy link
Member

@mathias-luedtke mathias-luedtke left a comment

Choose a reason for hiding this comment

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

This is not working at all. That's why the tests fail.

local err=0
local path
ici_make_temp_dir path
function install_clang_format_check() {
Copy link
Member

Choose a reason for hiding this comment

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

This function never gets called..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, thanks for pointing this out - I made a wrong assumption about the calling order. I've now simplified it. It works by e.g. setting BEFORE_RUN_CLANG_FORMAT_CHECK="wget [my config] -O /tmp/clang_format_check/[project]/.clang-format" along with ADDITIONAL_DEBS=wget in this case.

To download a custom config file, use
BEFORE_RUN_CLANG_FORMAT_CHECK="wget [my_config] -O
/tmp/clang_format_check/[project-name]/.clang-format".
You also need to set ADDITIONAL_DEBS=wget
@wxmerkt
Copy link
Contributor Author

wxmerkt commented Jan 21, 2022

Friendly ping - any thoughts on this?

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.

2 participants