-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat(dep): check integrity #1054
Conversation
This looks pretty great, thanks! I like this approach a lot, I think it makes sense. I had some comments but ShellCheck summarises them better, I'll leave them as comments on the specific lines themselves. I realise you had issues with the Bash LSP so it's understandable. As a workaround you can instead download ShellCheck 0.8.0 manually and add it to your path as an alias, that's what I do for ShellCheck 0.8.0. Then from the project folder, you can run Apart from that, I was wondering if we should make it a dedicated function, in case we can use this elsewhere, but I don't think we really use many native binaries this way. If needs be in future, we can split it out. Thanks for your contribution! Once the ShellCheck comments are addressed, I'll merge this :-) |
Yeah I am using Scite that doesn't use LSP and doesn't lag with huge files (I use that for mysql dump that often are bigger than this). |
ShellCheck v0.10.0 is out, and once I add the directive to not use extended analysis, as long as you're using an up-to-date LSP things should be fine. |
Sorry, I just realised I forgot to mention (and document) that the version should be bumped. There's no exact rule on this, basically just the date for you + 1, and then bump the revision number. So for example you might set This isn't enforced, this is just my own workflow that you might find useful when working on PRs: typically what I do is set the version during development, and in brackets, add the branch name. This means if another PR uses the same date, the version string is still unique. So in this case it might be This is used to know when and how to update config files, and they are assigned a version. It's not massively important for afaik any of your PRs but still just good practice (I forget to do it sometimes myself!) Once this is done, the rest of the changes look good to me, and this will be ready to merge. ShellCheck v0.10.0 is very angry about a lot of this script, but v.0.8.0 gives no warnings after the changes. At a glance though I think v0.10.0 isn't complaining about anything specific to this PR. Thanks a whole lot! 😄 |
As you asked version bumped. |
Thanks! Although the string isn't formatted quite right. Your version string is
This PR has conflicts because of my mergings, but I'll see if I can rebase this PR myself and merge it if that's ok. 😄 You can also use the diff here as reference for future PRs. Thanks! |
Fixed the version and for conflicts, yeah it is better that you do so :-) |
Heh, I took a quick look at how to contribute to someone else's PR using this gist. I will try my best not to mess anything up on you, but if I do, I can probably just pick your PR into another branch and then squash-merge it (you might end up as a co-author), but you'll be credited correctly in the changelog 😄) |
To me is perfectly fine :-) |
I got it in the end I think. Just doing one last ShellCheck run and then I will merge this. Thank you for your patience, continued discussions and contributions! It is much appreciated. |
Changelog has also been updated to include your contribution (might be easiest to see with Ctrl+F for Also, it looks like despite the commits in the PR being co-authored (because of the rebase), the squash-merge properly only included you, which I am happy about! I am very keen on making sure all contributors get properly recognised, which is also why I bring it up here too. Thanks again for all your work! |
Based on #859
This check if innoextract and cabextract are fine.
Check the size is not something that can confirm as you suggested in the ticket, the only one is to run the command and see if crashes.