-
-
Notifications
You must be signed in to change notification settings - Fork 26k
Have Travis ensure that what's new is updated #10451
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
Comments
Since that I am the first one to forget about it, it would be awesome (until the point that I want to kill Travis to throw those errors in addition of the flake8 one :P). Indeed, I am not sure 100% which case we are supposed to do a what's new entry or not. The flake8 build could serve for such checking I think. |
I think since it would be very fast, a separate build is worthwhile so we can see a separate tick or x for it... but I'm okay with it being in flake8 build too.... |
I'm suggesting the relationship changed tests -> what's new under the
assumption that we must report changed behaviours in what's new, and
changed behaviours must be reflected in changed tests (which we're much
better at requiring than what's new).
…On 11 January 2018 at 11:29, Guillaume Lemaitre ***@***.***> wrote:
Since that I am the first one to forget about it, it would be awesome
(until the point that I want to kill Travis to throw those errors in
addition of the flake8 one :P). Indeed, I am not sure 100% which case we
are supposed to do a what's new entry or not.
The flake8 build could serve for such checking I think.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#10451 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz6z9e-atoE5blNfrO1S2cAdUtkDCbks5tJVXygaJpZM4RaJLV>
.
|
+1 |
The slightly annoying thing with having a build failure if you don't add a whats_new entry is that they tend to be added once the PR is ready to merge, because it is easier to write a good summary then. Having a build failure from the beginning would probably lead to write a not so great whats_new entry until the PR is ready to merge. At this point, it is easy to forget to check the whats_new entry ... |
yes, I realise that, but think it's worth trying and seeing if having
what's new from the start helps in reviewing and if we get used to editing
it towards the end
|
what would danger do to notify us of a missing what's new?
|
shrug. I'd rather just try one or other solution and wait to see what we
think
|
Danger looks pretty nice. It general (also with respect to #10446) it might be useful to have some mechanism of performing automated actions based on some events. Another similar application I can think of how to handle stale PR. One could imagine that if a PR was reviewed, and there wasn't any response from the author, it would ping him/her in a comment and with no response after some amount of time (e.g. 6 months) mark the PR as in need of contributor. The issue that some reviews are in fact implicit rejections (or a consensus isn't reached), and that's harder to detect automatically. In any case, one can't do this with danger, unfortunately, I think.. |
Take |
Sometimes when PRs are merged I waste time going back to double check that what's new has been updated.
We should make a travis run for PRs only that fails if tests are modified and what's new isn't modified. Perhaps we should even check that the diff to what's new mentions the current PR# (although there are sometimes exceptions to this).
Does this seem a reasonable policy?
The text was updated successfully, but these errors were encountered: