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

Use system installed clang-format only if newer, or bundled version is not executable #4968

Merged
merged 7 commits into from
Feb 14, 2020

Conversation

Colengms
Copy link
Contributor

If a path to clang-format is configured explicitly, we'll use it. Otherwise, we will check for one on the system. If we find one on the system, we will:
- Try to invoke our own, to see if we can successfully do so, and to query its version.
- If we fail to invoke our own, use the one found on the system.
- If we successfully execute our own, try to invoke the one found on the system as well, to compare versions.
- If it fails to execute, use our own.
- If both are executable, use the one with the greater version.

This should address: #4963

Copy link
Contributor

@sean-mcmanus sean-mcmanus left a comment

Choose a reason for hiding this comment

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

Can you update the package.json version to 0.27.0-master?


// Invoke the version on the system to compare versions. Use ours if it's more recent.
try {
let output: string[] = execSync("\"" + path + "\" --version").toString().split(" ");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the preferred style is

`"${path}" --version`

unless it's a single string concatenation.

@Colengms Colengms merged commit 764b1d1 into master Feb 14, 2020
@Colengms Colengms deleted the coleng/check_clang_format_version branch February 14, 2020 21:29
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants