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

Feat: 1639 release process for minor versions #1657

Merged
merged 10 commits into from
May 14, 2024

Conversation

jcpitre
Copy link
Contributor

@jcpitre jcpitre commented Jan 24, 2024

Summary:

Added some documentation about the branching and release strategy.

Copy link
Contributor

✅ Rule acceptance tests passed.
New Errors: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
1 out of 1481 sources (~0 %) are corrupted.
Corrupted sources:
pl-pomorskie-zarzad-transportu-miejskiego-gdansk-ztm-gdansk-gtfs-1014
Commit: a883fe2
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

@emmambd emmambd self-requested a review January 24, 2024 14:03
@emmambd emmambd linked an issue Jan 24, 2024 that may be closed by this pull request
@jcpitre
Copy link
Contributor Author

jcpitre commented Feb 1, 2024

@emmambd You can review now.

## General Considerations

- Generally the master branch is the one where current development is happening.
- One objective would be to arrive to a point where the master branch can be released very often (every day?).
Copy link
Contributor

@emmambd emmambd Feb 1, 2024

Choose a reason for hiding this comment

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

@jcpitre It should be measurable what the goal is here (and we should commit to it in the text). How's once every two weeks, given that we're not always working on the validator? Or do we want to be decisive about daily? @davidgamez

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the section about releasing often or daily

Copy link
Contributor

github-actions bot commented Feb 1, 2024

✅ Rule acceptance tests passed.
New Errors: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1480 sources (~0 %) are corrupted.
Commit: 3b85b96
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.


## Release numbering
- example 4.3.0
- The first number (4 in this example) is incremented for major changes in the software. What major change mean has to be decided.
Copy link
Contributor

@emmambd emmambd Feb 1, 2024

Choose a reason for hiding this comment

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

Major = impacts the rules, either errors or warnings OR includes breaking changes for users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So do you mean adding a rule for example will require a major number change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct - any additions or modifications to validation rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How often are the rules changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would expect at the once every 3 month cadence we talked about (we're talking about modifying equal_shape_dist_traveled_diff_coordinates and trip_distance_exceeds_shape_distance this quarter, for instance, and David's expired_calendar PR would be a rule modification). We also need to add new fares rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that means we would never really have minor releases, if there are changes of rules that often. Can we qualify the changes in the rules to make some more important (and impactful) than others?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jcpitre I think the reaction would just be to release more :) For example, we could do a minor release for everything we've merged this past month. (Not suggesting it, but going forward after this quarter)

## Release numbering
- example 4.3.0
- The first number (4 in this example) is incremented for major changes in the software. What major change mean has to be decided.
- The second number (3 in this example) is incremented for minor changes in the software.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor = non-breaking changes for users, and don't impact validation rules.

- The first number (4 in this example) is incremented for major changes in the software. What major change mean has to be decided.
- The second number (3 in this example) is incremented for minor changes in the software.
- The third number (0 in this example) is used for maintenance releases.
A maintenance is created if some bug has to be corrected before the next minor release.
Copy link
Contributor

Choose a reason for hiding this comment

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

@jcpitre Can we link to an example? Such as this one: #1653

Copy link
Contributor Author

@jcpitre jcpitre Feb 1, 2024

Choose a reason for hiding this comment

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

We could, except we decided not to do a maintenance release with the PR you mentioned since there was a workaround that did not involve a release. So the example might not be as convincing as we want.

Copy link
Contributor

Choose a reason for hiding this comment

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

What would be an example you'd use here?

Copy link
Contributor Author

@jcpitre jcpitre Feb 1, 2024

Choose a reason for hiding this comment

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

Frankly I can't find any. The best is the one you mentioned. So I can put it in the document and add a comment in #1663 saying that this should lead to a maintenance release but we were lucky to have a workaround.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the link

- For example the change could be too extensive, or introduce an undue risk in the maintenance release.
This is true in particular if the testing infrastructure is not sufficient.

## Hotfixes
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be useful to explicitly state that we should label an issue as a "hotfix" issue, as discussed, so it's clear when a bug warrants this type of branching flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added something to that effect.

- For example if commit C from the diagram contains minor changes like typos we could decide that it's OK to include it in the maintenance release.
- In that case the hotfix branch can stem from commit C and be merged back into master in commit F.
- Commit F is tagged as the maintenance release and the maintenance release is built from there.
- If a merge on the master branch cannot be included in the maintenance release (See for example commit I) then we have no choice but to create a maintenance branch
Copy link
Contributor

Choose a reason for hiding this comment

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

@jcpitre So basically, if there's been a commit to master that would need to result in a major release version, then we have to create a maintenance branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is something we absolutely need to fix in the released software, yes.
Considering that normally you would not want the major commit to make it's way in the maintenance release.
But this can be true for minor commits too. Except it's not as clear cut.

Copy link
Contributor

github-actions bot commented Feb 1, 2024

✅ Rule acceptance tests passed.
New Errors: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1480 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
1 out of 1481 sources (~0 %) are corrupted.
Corrupted sources:
ci-abidjan-divers-operateurs-gtfs-913
Commit: 809cbc7
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.


- Feature/fix branch are created for every feature we want to add to the software (or bug we want to fix).
- They stem from master, are usually short-lived and are merged back to master.
- We already use these for adding features or fixing bugs in the master branch.
Copy link
Member

Choose a reason for hiding this comment

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

[suggestion]: delete this line, as the concept of when we are already using them becomes unclear after this PR is merged.

Copy link
Contributor

github-actions bot commented Feb 2, 2024

✅ Rule acceptance tests passed.
New Errors: 0 out of 1481 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1481 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1481 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1481 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1481 sources (~0 %) are corrupted.
Commit: 44e61b2
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

Copy link
Member

@davidgamez davidgamez left a comment

Choose a reason for hiding this comment

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

This LGTM. After this is approved, let's create an issue to align the release pipeline with the branching strategy.

Copy link
Contributor

✅ Rule acceptance tests passed.
New Errors: 0 out of 1520 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Errors: 0 out of 1520 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
New Warnings: 0 out of 1520 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
Dropped Warnings: 0 out of 1520 datasets (~0%) are invalid due to code change, which is less than the provided threshold of 1%.
0 out of 1520 sources (~0 %) are corrupted.
Commit: dae5e13
Download the full acceptance test report here (report will disappear after 90 days).
✅ Rule acceptance tests passed.

@jcpitre jcpitre merged commit 07ec502 into master May 14, 2024
333 checks passed
@jcpitre jcpitre deleted the 1639-release-process-for-minor-versions branch May 14, 2024 14:59
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.

Release process for minor versions
3 participants