Lokasi ngalangkungan proxy:   [ UP ]  
[Ngawartoskeun bug]   [Panyetelan cookie]                
Skip to content

Swift: Accept standardized CSV sink labels#13115

Merged
geoffw0 merged 3 commits intogithub:mainfrom
geoffw0:swift-csv-labels
May 11, 2023
Merged

Swift: Accept standardized CSV sink labels#13115
geoffw0 merged 3 commits intogithub:mainfrom
geoffw0:swift-csv-labels

Conversation

@geoffw0
Copy link
Copy Markdown
Contributor

@geoffw0 geoffw0 commented May 10, 2023

There's been an effort to standardize the CSV sink labels we use between different languages. This change adds the standardized labels for Swift (but does not remove the old labels - I'm open to discussion on whether and how we should do that, but personally I don't think they're doing a lot of harm).

@jcogs33

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Swift labels May 10, 2023
@geoffw0 geoffw0 requested a review from a team as a code owner May 10, 2023 17:29
@MathiasVP
Copy link
Copy Markdown
Contributor

I'm open to discussion on whether and how we should do that, but personally I don't think they're doing a lot of harm

I think Swift should embrace the fact that we don't have a deprecation period yet and just get rid of the old labels 😃. One way of doing things is better than two in this case, I think!

@jcogs33
Copy link
Copy Markdown
Contributor

jcogs33 commented May 11, 2023

I'm open to discussion on whether and how we should do that, but personally I don't think they're doing a lot of harm

I think Swift should embrace the fact that we don't have a deprecation period yet and just get rid of the old labels 😃. One way of doing things is better than two in this case, I think!

FYI, for Java I'm removing the old labels and including a change note on the PR. See #12916.

@geoffw0
Copy link
Copy Markdown
Contributor Author

geoffw0 commented May 11, 2023

OK, I've removed the old labels. I don't think we have any CSV sink models that use these particular labels, and we're not doing change notes yet, so that should be all that's needed.

@geoffw0
Copy link
Copy Markdown
Contributor Author

geoffw0 commented May 11, 2023

I don't think we have any CSV sink models that use these particular labels
...
FAILED: /Users/runner/work/codeql/codeql/swift/ql/test/query-tests/Security/CWE-312/CleartextLoggingTest.ql

Looks like I was wrong (I searched but my terms were a bit too narrow). Fixed now.

Copy link
Copy Markdown
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM once CI is happy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Swift

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants