Skip to content

Rust: remove shipped feature flag #2960

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Rust: remove shipped feature flag #2960

wants to merge 1 commit into from

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Jul 3, 2025

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@redsun82 redsun82 marked this pull request as ready for review July 4, 2025 14:41
@Copilot Copilot AI review requested due to automatic review settings July 4, 2025 14:41
@redsun82 redsun82 requested a review from a team as a code owner July 4, 2025 14:41
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

if (
config.languages.includes(Language.rust) &&
!(await codeql.resolveLanguages()).rust
!(await codeql.resolveLanguages()).rust // means codeql already supports rust without any intervention
Copy link
Member

Choose a reason for hiding this comment

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

Minor: I'd prefer this comment to be part of the comment above the if rather than at the end of this line, if you think that it is needed at all.

@@ -588,32 +587,25 @@ async function run() {
core.exportVariable(bmnVar, value);
}

// For rust: set CODEQL_ENABLE_EXPERIMENTAL_FEATURES, unless codeql already supports rust without it
// For rust, if running a version prior to public preview (2.22.1) set CODEQL_ENABLE_EXPERIMENTAL_FEATURES
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure that this comment accurately describes what is currently implemented. As far as I can tell you check whether the actual CLI version is less than minVer and if so throw a ConfigurationError. That means we never get to the CODEQL_ENABLE_EXPERIMENTAL_FEATURES check.

If that is the case, and that behaviour is unintentional, then you should obviously fix it, but I'd also like to perhaps see this refactored with a test to make sure it does the right thing.

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