-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Make Build models default to triggered
#8031
Conversation
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 like tests need to be updated, this also discovered a bug (just a missing none check) :D
btw, we already change the value to triggered
readthedocs.org/readthedocs/core/utils/__init__.py
Lines 118 to 125 in 655a34a
build = Build.objects.create( | |
project=project, | |
version=version, | |
type='html', | |
state=BUILD_STATE_TRIGGERED, | |
success=True, | |
commit=commit | |
) |
@ericholscher it would be good to finish this PR and merge it when you have some time. It seems there is some tests failing due to the change. |
@humitos feel free to finish it up if you have bandwidth. I don't think I'll have time for a little while, unfortunately. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I'm not sure why we were defaulting them to Finished, but this should help not confuse the badge code, and just be more correct in general.
417df20
to
181f9b7
Compare
I'm adding this PR to the Roadmap Q3 so we can finish it. It would be good to have our data being consistent. |
@stsewd would you like (if you have some time 😄 ) to take a look at this PR and update it so we can move forward and merge it? 🙏🏼 |
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.
Did a quick test locally, I didn't see anything weird.
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.
👍 Can't approve this since I originally opened it, but looks great. Thanks for pushing this to done!
I'm not sure why we were defaulting them to Finished,
but this should help not confuse the badge code,
and just be more correct in general.