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

add doc about wrappers and branching model #2003

Merged
merged 3 commits into from
Apr 13, 2020

Conversation

ptrovatelli
Copy link
Collaborator

@ptrovatelli ptrovatelli commented Mar 7, 2020

  • add doc aout wrappers
  • add doc about branching model

Copy link
Member

@valentijnscholten valentijnscholten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good starting point

@madchap
Copy link
Collaborator

madchap commented Mar 8, 2020

I'll review soon thanks :-) Meanwhile, can we agree to have file extensions as lower case? cheers :-)

@madchap
Copy link
Collaborator

madchap commented Mar 8, 2020

I am wondering if plantUML sequence diagrams could do here instead of draw.io (for the branching diagram) as to more easily be modifiable later. I can give it a shot.

@madchap madchap self-requested a review March 9, 2020 14:18
@madchap
Copy link
Collaborator

madchap commented Mar 12, 2020

I've tried to model the flow with plantUML so that we could have that as-code. I think it's a good start.
gist is at: https://gist.github.com/madchap/7bee5af2c1f9ce7e326ce8d8ae28d644

and the image produced is the following:
dojo_release_seq

@valentijnscholten
Copy link
Member

How are these backports/backmerges going to work? If every one of them needs a new PR with working travis and 2 reviews it's gonna take forever. I think we need 1 or 2 from us to be able to do this kind of maintenance, especially since the original authors aren't able to help us out here it seems (no offence, just observing).

@madchap
Copy link
Collaborator

madchap commented Mar 12, 2020

it's gonna take forever.

I think reviews are still needed somehow. Something to be discussed at our next meeting I guess.

@ptrovatelli
Copy link
Collaborator Author

@madchap thanks it looks actually nice!
I have added the "bug fixes" label that was missing and changed "4 weeks" to "4 to 8 weeks" (I think 4 weeks will be very tough);

https://gist.github.com/ptrovatelli/0ca8c68305398167d8fba9db7be33d0a

now "4 weeks" is behind another text. know how to fix that?

also you have removed the versions example but i was meant to show how versions change; we could replace 1.0.0 /1.0.1 by 1.x.y / 1.x.y+1 if you prefer?

@madchap
Copy link
Collaborator

madchap commented Mar 13, 2020

now "4 weeks" is behind another text. know how to fix that?

Just move your comment block below the "merge back" lines instead of having it above.

also you have removed the versions example but i was meant to show how versions change; we could replace 1.0.0 /1.0.1 by 1.x.y / 1.x.y+1 if you prefer?

OK, I thought it was just put there "like that", sorry. Well, I guess we'd have to define when there is major, minor. Patch or hotfix are fairly obvious I guess. Please add them back if you want, of course :) Sorry about that.

@ptrovatelli
Copy link
Collaborator Author

done

@madchap
Copy link
Collaborator

madchap commented Mar 13, 2020

@ptrovatelli Could you please lower case the file extensions? Cheers :)

I will approve also once you've updated the PR with the .puml and the png (or svg) :)

@ptrovatelli
Copy link
Collaborator Author

a couple more minor changes:

  • reword "back port" to "merge" in all cases. the difference seems thin; merge will probably work in most cases, else cherry-pick
  • add version name evolution; note that
    • release starts at 0 for last digit
    • tags start with "R" to avoid naming conflicts with branches
  • add the mention "(increase x if non backward- compatible API change)" for semver

what do you think?
https://gist.github.com/ptrovatelli/0ca8c68305398167d8fba9db7be33d0a

@madchap
Copy link
Collaborator

madchap commented Mar 16, 2020

a couple more minor changes:

  • reword "back port" to "merge" in all cases. the difference seems thin; merge will probably work in most cases, else cherry-pick

OK.

  • add version name evolution; note that

    • release starts at 0 for last digit

OK, so we make it always x.y.z then.

  • tags start with "R" to avoid naming conflicts with branches

Lowercase 'r' maybe? :-)

  • add the mention "(increase x if non backward- compatible API change)" for semver

OK. I guess we have not had any of this occuring in the past?

what do you think?
https://gist.github.com/ptrovatelli/0ca8c68305398167d8fba9db7be33d0a

Looks good to me.

Don't forget to remove your xml files and add the puml and related generated as well :-)

Thanks @ptrovatelli !

@madchap
Copy link
Collaborator

madchap commented Mar 17, 2020

Should we also add time-related references on the diagram for the beginning and end of a release and rc branch?

E.g. last week-end of the month, 2nd Tuesday of the month or something like that to give a time dimension there? Would also be useful clarify that dimension.

@ptrovatelli
Copy link
Collaborator Author

Should we also add time-related references on the diagram for the beginning and end of a release and rc branch?

E.g. last week-end of the month, 2nd Tuesday of the month or something like that to give a time dimension there? Would also be useful clarify that dimension.

i'm afraid that would be a bit ambitious don't you think?

@ptrovatelli
Copy link
Collaborator Author

OK, so we make it always x.y.z then.

starts at x.y.0 then in case of hotfix x.y.1, x.y.2...
this tells that new releases will always be x.y.0 (1.5.4 cannot be a new release for example)
This is open to discussion of course, just a proposal
We could add a fourth digit too if we come to have too many releases

Lowercase 'r' maybe? :-)

I don't have a strong opinion... on the diagram it would seem to close looking to x but in real life it'll be like r1.0.0 I think I could live with it... what's so wrong with capitals though? :)

@valentijnscholten
Copy link
Member

I would start simple and stick to semver as much as possible. First let's see if we can keep the release cadence, can approve PRs in time, etc.

@ptrovatelli
Copy link
Collaborator Author

ptrovatelli commented Mar 18, 2020

  • add the mention "(increase x if non backward- compatible API change)" for semver

OK. I guess we have not had any of this occuring in the past?

yeah well; this is just the exact wording from the semver official website @ https://semver.org/; i'm wondering if we shouldn't change this for any general major breaking change in the application.
it would go like:
x.y.z

  • increase z for fixes
  • increase y for adding of functionality
  • increase x for breaking changes.

x would often increase but it's not really a big problem (we can go over 10, even over 100 if required)

@ptrovatelli
Copy link
Collaborator Author

pushed the discussed changes (small r, wider definition of breaking change, small changes in the branching_model.md text itself, added .plantuml and it's screenshot)

would be nice to have the owners opinion on all this.

@devGregA @aaronweaver @mtesauro ?

@madchap madchap merged commit 4667a4e into DefectDojo:dev Apr 13, 2020
@mtesauro
Copy link
Contributor

I'm late to the party but for what it's worth, I am good with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants