8000 Have Travis ensure that what's new is updated · Issue #10451 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

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

Closed
jnothman opened this issue Jan 11, 2018 · 12 comments · Fixed by #19155
Closed

Have Travis ensure that what's new is updated #10451

jnothman opened this issue Jan 11, 2018 · 12 comments · Fixed by #19155
Assignees
Labels
Build / CI Easy Well-defined and straightforward way to resolve

Comments

@jnothman
Copy link
Member

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?

@glemaitre
Copy link
Member

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.

@jnothman jnothman added Easy Well-defined and straightforward way to resolve help wanted labels Jan 11, 2018
@jnothman
Copy link
Member Author

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....

@jnothman
Copy link
Member Author
jnothman commented Jan 11, 2018 via email

@glemaitre
Copy link
Member

changed tests -> what's new

+1

@lesteve
Copy link
Member
lesteve commented Jan 15, 2018

With heavy bias from #10446, danger seems to be able to do that.

@lesteve
Copy link
Member
lesteve commented Jan 15, 2018

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 ...

@jnothman
Copy link
Member Author
jnothman commented Jan 15, 2018 via email

@jnothman
Copy link
Member Author
jnothman 8000 commented Jan 15, 2018 via email

@lesteve
Copy link
Member
lesteve commented Jan 16, 2018

As far as I understand, you can write rules about what danger is supposed to check and then the danger bot comments on the issue with some summary (looks like there is only one bot comment at any single time, not sure I have not thoroughly checked).

Here is an example comment from their doc:

@jnothman
Copy link
Member Author
jnothman commented Jan 16, 2018 via email

@rth
Copy link
Member
rth commented Jan 25, 2018

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..

@cmarmo
Copy link
Contributor
cmarmo commented Jan 11, 2021

Take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build / CI Easy Well-defined and straightforward way to resolve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
0