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

Extract and don't mangle User Args. #200

Merged
merged 2 commits into from Apr 4, 2021

Conversation

thebeline
Copy link
Contributor

@thebeline thebeline commented Apr 1, 2021

The current implementation mangles the contents of userArgsNames and prevents the successful checks that follow (preventing setting out-format and any of the new- parameters. This fix extracts the names with a regex without mangling them. The original iterative extraction looked creative and mildly elegant, but in testing, expanding it to not mangle the names made it less elegant, so using a regex and doing it in one go without iterating the array seemed like a simpler and more straight-forward approach.

Resolves: #199

Note: It looks like this bug has been in here for about 10 months (10cbc92), fixing this may break existing deployments. Especially considering how I just suggested using this bug as a use-case fix.

@thebeline
Copy link
Contributor Author

thebeline commented Apr 1, 2021

Tested and behaviour confirmed: https://github.com/thebeline/OctoScreen/pull/6/checks?sha=59216435d4ad8a7101eab75378f31597bd506a56

Build failing because I did not use prettier JS. I am not familiar with that tool, and I am not sure how to resolve that.

@ldez
Copy link
Member

ldez commented Apr 3, 2021

Build failing because I did not use prettier JS. I am not familiar with that tool, and I am not sure how to resolve that.

You just have to run npm run format

"format": "prettier --write **/*.ts",

src/run.ts Outdated Show resolved Hide resolved
@ldez ldez force-pushed the BF/PreventFormatUserArg branch from 180182d to 92c027d Compare Apr 4, 2021
@ldez ldez added the bug label Apr 4, 2021
@thebeline
Copy link
Contributor Author

thebeline commented Apr 4, 2021

Build failing because I did not use prettier JS. I am not familiar with that tool, and I am not sure how to resolve that.

You just have to run npm run format

"format": "prettier --write **/*.ts",

I've got that sorted now. I guess that's SOP, and stuff, but it might get mentioned in the development section of the readme.

PS: Your ESLint settings are ruthless... 🤣🤣

ldez
ldez approved these changes Apr 4, 2021
@ldez ldez merged commit 5c56cd6 into golangci:master Apr 4, 2021
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants