-
Notifications
You must be signed in to change notification settings - Fork 21
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
Release #40
Release #40
Conversation
These releases represent an upstream release and not a release of the product. | ||
|
||
- Ensure you are on the master branch | ||
- Ensure git is not in a dirty state (ie stuff needs to be committed ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add that we should run integration tests first?
I am hoping, that the level of integration testing that would be suitable for comunity releases would happen in CI, but that is not the case yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not have been merged to master if the tests were failing so I see little benefit in re running them again ping @witmicko
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, we should in future run tests before release and err our if any of them fail.
In discussion with @AdamSaleh we believe it would be better to let the integration tests run as part of the PR and for this to happen organically rather than calling it out specifically in the doc |
Describe what this PR does and why we need it:
Changes proposed in this pull request