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

Add a check for missing swiftlint_version: key #139

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Jan 15, 2025

Add a check for missing swiftlint_version: key, so that if it's missing we exit gracefully instead of having docker run error complaining about invalid syntax

[Internal Ref]: This came up as part of p1736899091552619/1736896890.515319-slack-CC7L49W13


  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

So that if it's missing we exit gracefully instead of having `docker run` error complaining about invalid syntax
@AliSoftware AliSoftware requested review from iangmaia and a team January 15, 2025 10:34
Copy link
Contributor

@iangmaia iangmaia left a comment

Choose a reason for hiding this comment

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

👍

@AliSoftware AliSoftware enabled auto-merge (squash) January 15, 2025 10:46
@AliSoftware AliSoftware merged commit 7444511 into trunk Jan 15, 2025
13 checks passed
@AliSoftware AliSoftware deleted the run_swiftlint/detect-missing-version branch January 15, 2025 10:47
@ParaskP7
Copy link
Contributor

ParaskP7 commented Jan 15, 2025

👋 @AliSoftware , I am seeing that you are updating the README and CHANGELOG, I did the same as part of this new #138 PR of mine. Ah... this is now merged, I wasn't fast enough! 😅

PS: Apologies, but I wasn't aware about this README and CHANGELOG process before, #135 being my first PR and all.

@AliSoftware
Copy link
Contributor Author

👋 @AliSoftware , I am seeing that you are updating the README and CHANGELOG, I did the same as part of this new #138 PR of mine. Ah... this is now merged, I wasn't fast enough! 😅

PS: I wasn't aware about this README and CHANGELOG process before.

The update of the CHANGELOG is documented in the PR's template (see checkbox).

The README update I did only because the plugin linter was complaining about it. Having to update that one regularly every time we do a new version is kinda annoying tbh (but Buildkite's official plugin linter job that we use to check this plugin checks for it so it's kinda forcing us to keep it in sync like that)

@ParaskP7
Copy link
Contributor

The update of the CHANGELOG is documented in the PR's template (see checkbox).

Yea, apoligies for missing that, I kind of got used to just check these boxes without really paying close attention to them, working on multiple client/library projects that have a tons of them... 🤷 I should be careful with the tooling projects however, especially ones that I haven't contributed just yet... 😊

The README update I did only because the plugin linter was complaining about it.

Yea, I know, I got the same while working on #138 . 🫣

Having to update that one regularly every time we do a new version is kinda annoying tbh (but Buildkite's official plugin linter job that we use to check this plugin checks for it so it's kinda forcing us to keep it in sync like that)

Noted! 👍

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.

3 participants