-
Notifications
You must be signed in to change notification settings - Fork 98
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 GitHub Action workflow to validate/format Rock-ons #348
Conversation
@StephenBrown2 this looks pretty cool! |
@StephenBrown2 This is really nice. Well done. Go you say: look at you! I've been meaning to familiarise myself with go for quite a while now so this is quite intriguing.
Yes I think that would be preferred, and if it is to be integrated into our workflow it would have to be really. So if you would be good with that, that would be dandy. Which brings up the license question: rockstor-core is GPL-3.0-or-later, but our Rock-on definitions are AGPL-3.0-or-later. However I'm unfamiliar with go, does its runtime have additional license burdens? See: https://github.com/rockstor/rockstor-core#readme @FroggyFlox & @Hooverdan96 the function here re GitHub Actions & Rock-ons is more in both of your areas as it goes so I'll mark for review and have to leave that review to either/both of you I'm afraid. But if we are to integrate this rather nice mechanism it must have a license, as all our stuff has to be licensed, that is: FSF Free/Libre & OSI approved It's a requirement of our Open Collective and down to the nature of the project as a whole. And we have now standardised on https://spdx.org/licenses/ licensing identifiers as they are machine readable. So no license is a deal breaker for inclusion of any type, and a non FSF Free/Libre & OSI approved license is also a no go: as it goes. Excellent work this thought and something we would really use. Thoughts. It would be great to have a validator - if licenses and org hosting allow that is. |
I don't know if it is a burden, but the Go source files are distributed under the BSD-style license found in the LICENSE file. There are a couple libraries in use which either use the above or MIT licenses. I'll happily license it as |
OK, from that license link we have that as a: Which is both FSF Free/Libre & OSI approved so we are good on the non-encumbered front re bundled run-times (the go interpreter presumably).
MIT license version may be important: i.e. The Apple MIT is neither FSF Free/Libre or OSI approved (who would have thought). The plain SPDX 'MIT' is the only one that is both FSF Free/Libre & OSI approved: https://spdx.org/licenses/MIT.html All other MIT variants listed on that SPDX link: https://spdx.org/licenses/ are neither or only one side approved. I know it's a drag but our licensing position is super important and I for one cannot contribute, or host as the maintainer under an Open Collective, if it's not at least clear what we are intending to use with all due diligence to clarify what we are building with. Your program, as a hosted service scirts many requirements: but we may one day want to have it as a bundled option: or as a Rock-on that we host for example. To assist folks before they submit to the rockon-registry. Just a thought. Thanks for persevering with all this. It just took me months to get all our licensing clear and expressed everywhere in SPDX approved notation and I'd hate to back-slide and risk the entire project's new Open Collective standing.
Excellent, thanks for the flexibility on that front. However: as a GitHub Action surely it would be the definition of a network service no? I guess those outside the Org are not accessing it via an API: but the Org would be. Licensing is so exciting is it not! @FroggyFlox @Hooverdan96 |
Go, being a statically compiled language, doesn't have an "interpreter", so that should easy. The library in question (which is strictly to color the logs and can totally be removed if needed), appears to use the "plain" MIT license. TBH I didn't even know there were multiple MIT licenses. I think we are in the clear to use the GPLv3+, but I am still curious about the idea of the CLI script being a "hosted service". It's GitHub Actions that's the service, the CLI is just that: a CLI tool, intended for use as you said to assist folks before they submit to the rockon-registry (If approved, I would note it in the README as a step for the contributor to run before even opening a PR). It has no network interface (AGPL applies primarily to software that is used to provide service over a network) and the AGPL states:
So, in fact, I would think that the licenses should be flipped; in that rockstor-core would need the AGPL, as being primarily (only?) available as a network server, and the rockon-registry, being only useful if the actual source is available, being under the GPL. Or, since they are not "code" or even close to a "service", but rather "configuration", a simpler license such as Creative Commons, or the zlib license (which is, in fact, both FSF Free/Libre & OSI approved!) would be more appropriate. Boy, licensing sure is fun! :crossed-eyes: |
I think it's a great idea, if/when all of the licensing hurdles are out of the way. |
@StephenBrown2
We do not host rockstor - it is self hosted. But we do distribute it. But interesting point yes : however we are way into our existing license, so I think we are pretty much set there. Especially with so many contributors etc. However I'd really like to drop our own dependency count - but that is, as always, ongoing :).
Ohh we are getting into some strange epistemological territory there :) . What is code if it is not a configuration (read data) to be interpreted/compiled via a syntax that includes a grammar (making it a language). Just a thought. The main reason to have our Rock-on configs under AGPL is so that if they are hosted by others - they are obliged to contribute modifications back to the community at large. As they would be making them available via a network: i.e. the internet. I hope we are done this this license chit-chat :) it really isn't my favourite part of things: but it is critical to the survival of open source projects and for folks to share/benefit from whatever improvements have been made: i.e see OSX or whatever it's call now, vs BSD that it was branched from. One has benefited from the other - but not the other way around. Ergo my lack of enthusiasm for the slacker licenses. Our docs are Creative commons by the way. But anything code (I know!!) is not recommended to be under Creative Commons. I got that impression of late from an article or podcast that I'm unfortunately unable to resource. Sorry. Good to have you back contributing by the way. |
Yes, but the purpose of the license is not to restrict the producer, but rather anyone that might take the code and offer a derivative, for example if I were to offer Rockstor instances as a hosted service in my hypothetical data center... But I don't mean to drag the conversation on. While I disagree with some of the interpretations you've made of the licenses, IANAL and have no need to discuss it further. You'll notice I've added the appropriate LICENSE, and am ready to transfer the repository to rockstor the whenever you can give me access to, or however you'd like to handle that. I would of course like to continue working on it based on feedback that I anticipate will arise. Speaking of said feedback, shall I open another PR here with the formatting changes from running the validator now?
Oh, and thanks! |
@StephenBrown2
No lets have this all done as issue/pull-requests sets once it's in the org repo. And on that point. I'll open a new repo and you can then do a pull request to that repository in the org: if that works for you. You will then already have your fork (original) of it to do pull requests from. Hopefully that makes sense, getting a little later in the day here now. Thanks for your flexibility and understanding on all this by the way. I agree with the point re rockstor-core, of course, but we are a long-in-the-tooth project on that front. Our Rock-ons are a relatively new capability as it goes. And changing licenses can be quite the ordeal once code is already release under an initial license. Cheers. Keep an eye out for the new repo - assuming @FroggyFlox is also good with this approach. |
Hey @FroggyFlox, I would prefer to transfer (rather than fake-fork as @phillxnet suggested) the repository, as I think that will be the cleanest method. You would need to give me permissions to create repositories briefly while I did the transfer, and then you can revoke that permission once it's done, since I would be added as a collaborator after the transfer. Information from here: https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository
Ready to hit the button, as soon as you say go! |
@StephenBrown2 I'm really not happy giving repo creation access to an org that I am the owner on. What is wrong with populating vai a pull request, you have no issues pull requests collaborators/contributors. And would you not be a contributor (the only one likely) post the initial meger. What am I missing. There is no fake-fork: we create a repo in org and you populate it without gaining admin in the org: even for a millisecond. I'm likely missing something here sorry. But we cannot give org rights away willy nilly. Some cannot there-after be revoked you understand. We are kind of in a delicate position here: and as Org owners we state what we are happy with: I'm not happy given repo create access I'm afraid: that seems like a little bit of a jump from where we are now no? |
This comment was marked as outdated.
This comment was marked as outdated.
Oh! I just had another idea. If @FroggyFlox is handling the repository management anyways, I could simply transfer the repo to them, and they can transfer it to the org. If that is acceptable, go ahead and confirm the transfer email that was just sent to your (@FroggyFlox's) account, and no permissions or roles need changing at all! Apologies for the long-winded comment above, I'll admit I had only thought of the two possibilities, and not considered the indirect but permission-less option. |
@StephenBrown2 Re:
That's a good idea. I'd not thought of that. We have only created new repos in the org and have repo create disabled even for member. The org is used less as a work place and more as a central repository, with folks working-in & committing from their own forks via pull requests only, non of us direct push to org repos unless it's an emergency. My apologies for my tone in my last post: it had been a long day; and org permissions are somewhat of a worry for me as it goes. Thanks again for helping out here. I'll have to embark on my long planed Go pick-up soon then :). |
Hi @StephenBrown2, sorry for taking so long to get back to you... my schedule is a bit erratic at the moment. Thanks a lot for all of this, by the way! I'll share more elaborate feedback as soon as I have a bit more time (hopefully later today, Eastern time). Sorry for such a rushed input on my end for the moment. |
No worries on the time @FroggyFlox , I had a busy weekend myself. Thanks for taking care of the transfers! |
Closing as pending new repo developemnt. |
Greetings again, I've got a new toy for you to try. I'd been wanting some way to make sure that rock-ons were formatted in a similar way, and that everything was present that needed to be, and nothing that wasn't. So I wrote a tool in my new primary language: Go. You can find the repo here:
https://github.com/StephenBrown2/rockon-validator but I would just as well have it live under the rockstor org.The transfer has been made and the repository is now at https://github.com/rockstor/rockon-validatorGeneral information on project
Options
I gave it a couple options, namely
--check
, to simply exit non-zero if files need changing,--diff
, to output the changes that would need to be made to stdout, and--write
, to actually overwrite the files with proper formatting and validation.GitHub Action
I've added it as a GitHub Action for this repo so we can see how it works. Currently almost every file would need changes, which I can make in a different PR or this one, though a demonstration does exist in the README for the tool.
The GitHub action will run the tool over every file in the repo, and suggest changes based on what it would have overwritten. We could potentially add it as a pre-commit script or requirement in the Checklist that it be run before a Rock-on PR is accepted.
Features
Currently, the tool does the following:
root.json
file, assumed to be in the same directory as the file, or another path can be given.rock-on.json
and theroot.json
, and warns if they don't. I wasn't sure if we wanted to auto-correct so they match or in what direction if we did, since I think it might mess up the listing for installed rock-ons if the name changed?root.json
sorted alphabetically anyways (surprise, it isn't currently), I figured this was a reasonable trade-off and would enforce a black-like formatting for the JSON. NOTE: This also means that the "environment" map sorts by the environment variable name, rather than the "index" value, as they are currently sorted.Other Issues
While working on it, I noticed a couple things were not documented in the README, namely container_links and custom_config, but I've based the model off of what currently exists in the registry JSON, rather than trying to figure out what is an expected field and what isn't from the rock-on model.
Future Enhancements
Things the validator does not do, but might be able to:
Fixes #91
Checklist
root.json
in alphabetical order (for new rock-on only)"description"
object lists and links to the docker image used"description"
object provides information on the image's particularities (advantage over another existing rock-on for the same project, for instance)"website"
object links to project's main website