search

BlueSkyTools / BlueSkyConnect

An SSH tunneling solution for remote Mac-to-Mac connectivity (formerly a Mac-MSP product)
https://hub.docker.com/r/sphen/bluesky/
Apache License 2.0
10 stars 4 forks source link

Agree on shell code styling #28

Open smaddock opened 2 years ago

smaddock commented 2 years ago

I've been building a loose “style guide” for BlueSkyConnect shell scripts. I've been documenting it in a .editorconfig and you can see the changes in the main branch of my fork.

Some additional changes I want to make right away:

There should be no functional changes, just formatting, documentation, bug fixes, small optimizations, and very minor restructuring — but with 1200 lines and counting changed, it will definitely require testing.

If these kind of linting changes are of interest and people agree with my style choices, I can submit a PR, but I also know reviewing big PRs is a time suck, so it might be easier to start like a 2.4alpha branch or something. Whatever the community is interested in.

sphen13 commented 2 years ago

all sounds good to me - i would say there is a ton to review here. We could just create a dev or wip branch for commits we want against the next version. i would like to keep master/main down to latest release. when we are good with say the dev branch and ready to release we will PR to main and create a tag for the release.

smaddock commented 2 years ago

If you create the dev branch I’ll submit a PR against it when I have this at a good point. Maybe dev-2.x? To leave room for a future dev-3.x.

smaddock commented 2 years ago

Also definitely want @AllPurposeBen’s input on the styling choices.

sphen13 commented 2 years ago

done - dont get ahead of yourself with 3! :)

AllPurposeBen commented 2 years ago

I'm good with whatever standardization as long as it's documented for ref and/or editor config'd. else I'll forget.

Definitely a testing branch. I'd love to be able to unitary test via docker so if we can document how a dockerfile for getting from a branch to a working testing env, that would help with consistency, for me at least.

I've got stock testing VMs back to 10.11 for client testing, wasn't planning on going older than that. I'd even feel ok bumping the "we tested this X far back, YMMV" floor up to like 10.14.

smaddock commented 2 years ago

I think I have my shellFormatting branch at a good point to consider merging into the dev-2.x branch. I didn't change the reverse DNS since that could be a breaking change.

Since there are no functional differences I don't think it's worth a release, but I think it is a better point to move forward with actual modifications.

I still need to test it to make sure I didn't bork anything obvious and then I'll submit a PR.

smaddock commented 2 years ago

I have the first few rounds of code cleanup ready, but I think it'd be easier to wait until existing PRs are committed so we don't have merge conflicts.