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

Added clang v16 to README and action description #145 #147

Merged
merged 2 commits into from May 19, 2023

Conversation

shenxianpeng
Copy link
Collaborator

@shenxianpeng shenxianpeng commented May 19, 2023

I also removed 7, 6, 5, 4, or 3.9. from README.md to keep the same as it is in action.yml. Even though we support these versions, I think most users will not use them.

@shenxianpeng shenxianpeng added the documentation Improvements or additions to documentation label May 19, 2023
@shenxianpeng shenxianpeng requested a review from 2bndy5 May 19, 2023 04:43
@shenxianpeng
Copy link
Collaborator Author

Since we have supported clang v15 and v14, but the default version of clang is v12, do you think we should also update the default version a little higher? maybe v14?

@2bndy5
Copy link
Collaborator

2bndy5 commented May 19, 2023

I also removed 7, 6, 5, 4, or 3.9. from README.md ... Even though we support these versions, I think most users will not use them.

Older projects may need to stick with what was used when introducing static analysis. This is because the major versions have breaking changes between them. If we do support these versions, then they should stay in the README. Otherwise, it would be misleading or unappealing for those that still use those versions.

Since we have supported clang v15 and v14, but the default version of clang is v12, do you think we should also update the default version a little higher? maybe v14?

I don't really mind what the default is now that the version option is all but required. Perhaps, we should make it a required option to

  1. avoid having to keep updating the default
  2. ensure user-expected behavior

Although, I guess that would be a breaking change in the action.

@shenxianpeng
Copy link
Collaborator Author

If we do support these versions, then they should stay in the README. Otherwise, it would be misleading or unappealing for those that still use those versions.

Agree. so I should add 7, 6, 5, 4, or 3.9 to action.yml to keep confidence.

We should make it a required option

I don't know how to make it a required option, if the user doesn't input version causing action failed that might not be very nice.

Yes, changing the default value maybe cause a break, so I will leave it alone until the user highly suggests us or we make a major/minor not in patch release.

@2bndy5
Copy link
Collaborator

2bndy5 commented May 19, 2023

I don't know how to make it a required option

version:
description: "The desired version of the clang tools to use. Accepted options are strings which can be 15, 14, 13, 12, 11, 10, 9, or 8. Defaults to 12."
required: false
default: "12"

Its a one line change on our end.

@shenxianpeng shenxianpeng merged commit 14dc26d into main May 19, 2023
29 checks passed
@shenxianpeng shenxianpeng deleted the update-support-version branch May 19, 2023 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants