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

shellcheck: general linting and github action #763

Merged
merged 21 commits into from
Dec 16, 2024
Merged

shellcheck: general linting and github action #763

merged 21 commits into from
Dec 16, 2024

Conversation

yaazkal
Copy link
Collaborator

@yaazkal yaazkal commented Dec 9, 2024

This PR has two objectives:

  1. Make a shellcheck linting on every new PR.
  2. Clean the code using some suggestions of that linting in order to make the new PRs checks clean.

Relevant behavior that has been tested according to the changes:

  • bootstrap.sh: Bootstraping linux jail (debian or ubuntu) in aarch64.
  • clone.sh Clone VNET jail.
  • common.sh VNET jail creation.
  • convert.sh Convert a thin jail into a thick jail.
  • create.sh Name sanity to not allow special characters.
  • create.sh clone.
  • destroy.sh destroying a jail (stopped or using -f).
  • list.sh normal and using -a.
  • stop.sh OK.
  • export.sh conditional checks OK.
  • import.sh OK.
  • rdr.sh OK.
  • rename.sh OK.
  • service.sh OK.
  • template.sh RENDER command OK, CP command OK.
  • tags.sh OK.
  • update.sh OK.
  • verify.sh disabling some shellchecks here. OK.

Pending to test: mount

@yaazkal yaazkal changed the title updates version on README.md shellcheck: general linting and github action Dec 9, 2024
@yaazkal
Copy link
Collaborator Author

yaazkal commented Dec 9, 2024

Can you help me test this @bmac2 ?

@yaazkal yaazkal requested a review from bmac2 December 9, 2024 02:39
@tschettervictor
Copy link
Collaborator

What are you trying to accomplish with the templates command?
See PR #761

@yaazkal
Copy link
Collaborator Author

yaazkal commented Dec 10, 2024

@tschettervictor nothing special in the template.sh here, just making a genereal linting to all the project and touching as little as possible in this PR in order to merge it later and have the linter in all the future PRs.

By the way, those PRs will be after this one, but I don't want to merge this until is well tested.

@tschettervictor
Copy link
Collaborator

Great! Looking forward to it. You'll notice I pushed quite a few PRs in the last few days addressing some bugs and features.

@yaazkal
Copy link
Collaborator Author

yaazkal commented Dec 10, 2024

Great! Looking forward to it. You'll notice I pushed quite a few PRs in the last few days addressing some bugs and features.

Yes, thank you. I'll finish test this PR in order check those later this week/weekend

@tschettervictor
Copy link
Collaborator

Yes, thank you. I'll finish test this PR in order check those later this week/weekend

Anything I can do to help?

@yaazkal
Copy link
Collaborator Author

yaazkal commented Dec 10, 2024

Anything I can do to help?

Sure, thanks. If you want to test this PR and give us feedback, that will be a great help. Mainly I'm running all the commands that I've touched to ensure nothing is broken (without taking into account other PRs of course).

Mainly could be typos that can breake something.

@tschettervictor
Copy link
Collaborator

You're referring to the bastille commands correct? Will see if I can go ahead and do that.

@yaazkal
Copy link
Collaborator Author

yaazkal commented Dec 10, 2024

You're referring to the bastille commands correct? Will see if I can go ahead and do that.

yep, I've touched multiple bastille commands here, mainly quotes and syntax changes. I just want to be extra sure that nothing unwanted is going to happen later

@yaazkal yaazkal self-assigned this Dec 11, 2024
@yaazkal yaazkal added the enhancement New feature or request label Dec 11, 2024
@yaazkal
Copy link
Collaborator Author

yaazkal commented Dec 15, 2024

Just updated the description of this issue with my tests, also made some extra commits fixing previous commits after the tests. Only mount has been not tested from my side, but feel pretty confident about this PR now. @bmac2 if you are ok with this you can merge it or let me know next steps.

This PR is blocking the review/merge of other PRs.

@bmac2 bmac2 merged commit 0e6b723 into master Dec 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants