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

WIP: Commence develop branch starting with tojson & tplroot changes #147

Merged
merged 11 commits into from
Jul 8, 2019

Conversation

myii
Copy link
Member

@myii myii commented Jun 25, 2019

A few months back, I recall @aboe76 mentioning that we should be testing against the upstream develop branch as well. I got reminded of this by @5paceToast in a discussion starting from here. Thanks to @javierbertoli, we now have 5 "pre-salted" develop images, which I've helped to configure to ensure that they always pull the very latest develop commit for testing within the Travis matrix.

This PR is a fully-functional proposal to start a develop branch here in the template-formula, which allows us to work on cutting-edge PRs, that can later be merged back into the master branch when the relevant Salt versions are officially supported.

Some considerations before proceeding with this:

  1. Should this develop branch become this repo's main branch for all PRs and then merge backwards (in a similar vein to the upstream Salt repo)?
  2. Since PRs will be merged back to master at different times, should there actually be multiple branches representing each version of Salt as well (almost exactly like the upstream Salt repo)?
    1. In terms of this PR, then the tojson commit would actually belong in a 2018.3 branch whereas the tplroot commit would remain in the top-level develop branch.
  3. Should semantic-release be enabled for this/these branch/branches?
    1. As a note, the latest versions of semantic-release can actually provide support for very intricate semantic versioning schemes (including developmental releases).

CC (those not mentioned so far): @daks @noelmcloughlin @n-rodriguez @vutny @alxwr.

@CosmicToast
Copy link

My 2c re: 1 and 2 (feel free to ignore): I think simulating salt's structure is a good idea.
Different people will want to target different "minimal versions", and having a multi-branch (including develop) approach thus becomes the most useful for the most people (even outside of saltstack-formulas).
It would also help remember what features are getting merged when, allowing master (which has to support last N releases) to remain optimal.

@myii
Copy link
Member Author

myii commented Jun 25, 2019

@5paceToast That makes most sense to me as well but I want to ensure that maintainers are happy to go through with this, since it will require more effort when working with PRs.

@myii
Copy link
Member Author

myii commented Jun 25, 2019

Did I say 5 "pre-salted" develop images? Well, it started working on my fork:

But since it's still misbehaving, I've gone back to the 4 tried-and-tested develop images that have proven to be reliable.

@myii
Copy link
Member Author

myii commented Jun 25, 2019

Some initial discussions with @javierbertoli in our Slack/IRC/Matrix room:

- - -
17:51 myii @​gtmanfred There have been discussions about having a develop branch for the template-formula (and perhaps other formulas in the future). We're now at the stage to realise this with the 5 "pre-salted" develop images working as required. The first changes are proposed here: https://saltstack-formulas/template-formula #147. A couple of questions at this stage: (1) If all are in agreement, would it be OK to set the develop branch as the main branch on the repo? (2) Do you think it would be better to have branches tracking each Salt version, mimicking the upstream Salt repo?
17:57 javier I have mixed feelings with all these changes...
17:58 myii Sure, and that's a good thing.
17:59 javier oh, if that was the goal, I'm glad I helped and it succeeded! :)
17:59 myii Share your mixed feelings, it's important that we don't commit to something that we're unable to maintain.
18:00 myii @​javier BTW, I included that default-opensuse-leap-15-develop-py3 image because it started working in my fork, only to fail when running in the PR!!
18:01 myii It's temperamental...
18:03 myii But on this develop idea -- should we at least have a single develop branch? Or just use the develop images in the main matrix to make sure each PR is future-compliant?
18:04 myii Or can we at least experiment with the template-formula, to see how it goes when working with all of the branches?
18:06 javier on the develop/naming/branching issue, I feel that we're probably getting to a point where the template-formula is overblown for a newcomer (on one hand), so adding all these branches will complate things more ... and otoh, I think that 'develop' is a meaning which is usually intrinsic to a repo, meaning that dev happens there and what's merged to 'master' is the 'current state' of the repo. We have that with PRs. So in this case, what you're proposing is that 'develop' does not refer to the repo but to another repo's state (namely salt's repo).
18:09 javier So it feels to me like overloading the meaning, twisting it in an uncommon way that might surely complicate things. Ie, I'm a newcomer, want to submit a patch to the template-formula, my code is a fix for 20.18.3. Which branch should I submit against? "master"? I might probably pick develop, because I want it to go against 'the latest template-formula code', but that's not true for the name develop in this case, right?
18:10 myii Rough idea: all PRs through develop first.
18:10 myii Then rolled down the versions as appropriate by maintainers.
18:10 myii So contributors don't have to worry about anything other than the develop branch.
18:11 myii s/versions/branches ^^.
18:12 myii When pulling by tags, that will still be hitting the master branch, so no risk of getting recent (risky) features from the develop branch.
18:13 myii And then we can merge PRs with a little more panache, not always obsessing over how many installations we're going to break out in the wild.
18:14 javier as long as we keep this 'special naming situation' restricted to the template-formula, I can live with it. After all, template is a 'not many writes, lots of reads` kind of formula.
18:15 myii I reckon we could even fit in your small-medium-large ideas as well...

@CosmicToast
Copy link

I agree with javier - the "special naming situation" makes sense as a reference, but I don't see any real reason to make everything else be like that.
However, regarding this statement...

I feel that we're probably getting to a point where the template-formula is overblown for a newcomer (on one hand), so adding all these branches will complate things more

IMO it's already the case - deciphering the current map.jinja is not trivial for someone coming into this new (e.g me).
However, if anything, the develop branch will have the most convenience features (from latest salt), which will actually simplify the template itself.
Sure, all of the branches being present might be confusing, but the actual comments of the branches will end up being more clear, in my opinion.

@myii
Copy link
Member Author

myii commented Jun 26, 2019

@javierbertoli The comment above was some feedback related to our discussion in Slack. Thanks for that @5paceToast.

myii and others added 11 commits June 27, 2019 13:49
## [3.0.4](saltstack-formulas/template-formula@v3.0.3...v3.0.4) (2019-06-27)

### Documentation

* **contributing:** add recent `semantic-release` formulas ([22052fc](saltstack-formulas@22052fc))
## [3.0.5](saltstack-formulas/template-formula@v3.0.4...v3.0.5) (2019-06-28)

### Documentation

* **contributing:** add recent `semantic-release` formula ([fc50a9e](saltstack-formulas@fc50a9e))
refactor(string): remove capitalisation from 'template' string
## [3.0.6](saltstack-formulas/template-formula@v3.0.5...v3.0.6) (2019-06-28)

### Code Refactoring

* **string:** remove capitalisation from 'template' string ([7062210](saltstack-formulas@7062210))
## [3.0.7](saltstack-formulas/template-formula@v3.0.6...v3.0.7) (2019-07-04)

### Documentation

* **contributing:** add recent `semantic-release` formula ([c679cb5](saltstack-formulas@c679cb5))
## [3.0.8](saltstack-formulas/template-formula@v3.0.7...v3.0.8) (2019-07-08)

### Documentation

* **contributing:** add template-formula to `semantic-release` formulas ([87e4ebc](saltstack-formulas@87e4ebc))
@myii myii merged commit f2c6454 into saltstack-formulas:develop Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants