Skip to content

Conditionally skipped builds are "pending" instead of "passed" #9791

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.

Sign up for GitHub

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

Jump to bottom
Closed
benjaoming opened this issue Dec 8, 2022 · 3 comments · Fixed by #9807
Closed

Conditionally skipped builds are "pending" instead of "passed" #9791

benjaoming opened this issue Dec 8, 2022 · 3 comments · Fixed by #9807
Assignees
@ericholscher
Labels
Bug A bug

Comments

@benjaoming
Copy link
Contributor

PRs end up having the "pending" state although a build has been correctly and intentionally skipped.

Example:

image

image

image

#9789
https://readthedocs.org/projects/dev/builds/18859457/

In another PR, the build wasn't skipped even though it seems to have contained no changes:

#9734
@benjaoming benjaoming added the Bug A bug label Dec 8, 2022
@benjaoming benjaoming mentioned this issue Dec 8, 2022
Merged
@humitos
Copy link
Member

humitos commented Dec 8, 2022

I've seen the same behavior lately, but I think it's related to some GitHub permission because every time a PR without changes in docs/ is opened (the build is skipped) I got this notification in my dashboard:

Screenshot_2022-12-08_19-40-58

@benjaoming benjaoming mentioned this issue Dec 12, 2022
Merged
@humitos humitos moved this to Planned in 📍Roadmap Dec 13, 2022
@benjaoming
Copy link
Contributor Author

We need to track the GitHub settings for "Required Builds" here.

The PR referred here #9788 will make it impossible to use required builds together with skipped builds AFAICT. The skipped builds will turn out yellow and the PR will never be merge-able.

@ericholscher
Copy link
Member

I am debugging this now, and think I've found the issue.

@ericholscher ericholscher mentioned this issue Dec 13, 2022
Merged
ericholscher added a commit that referenced this issue Dec 15, 2022
@ericholscher
Fix status reporting on PRs with the magic exit code
0e2f770 
This was because we were using this logic to return a relative URL,
and then sending it to GitHub:

* https://github.com/readthedocs/readthedocs.org/blob/f86b5376ffc70242bdd677cb02f016429ff6bd4b/readthedocs/oauth/services/github.py#L459-L460
* https://github.com/readthedocs/readthedocs.org/blob/f86b5376ffc70242bdd677cb02f016429ff6bd4b/readthedocs/builds/models.py#L343-L349

Fixes #9791
@ericholscher ericholscher moved this from Planned to In progress in 📍Roadmap Jan 4, 2023
@humitos humitos moved this from In progress to Needs review in 📍Roadmap Jan 31, 2023
@github-project-automation github-project-automation bot moved this from Needs review to Done in 📍Roadmap Feb 2, 2023
humitos added a commit that referenced this issue Feb 2, 2023
@ericholscher @humitos
Fix status reporting on PRs with the magic exit code (#9807)
2b33b0f 
* Fix status reporting on PRs with the magic exit code

This was because we were using this logic to return a relative URL,
and then sending it to GitHub:

* https://github.com/readthedocs/readthedocs.org/blob/f86b5376ffc70242bdd677cb02f016429ff6bd4b/readthedocs/oauth/services/github.py#L459-L460
* https://github.com/readthedocs/readthedocs.org/blob/f86b5376ffc70242bdd677cb02f016429ff6bd4b/readthedocs/builds/models.py#L343-L349

Fixes #9791

* Fix link_to_build defaults

* Fix tests

* Comment on usage

* Fix test

* Use same logic as absolute_url

* VCS commit status: send absolute URL to docs when built successfully

- remove `link_to_build` since it's not required
- base the decition on `Build.state` and `Build.success`

---------

Co-authored-by: Manuel Kaufmann <[email protected]>
@ni-jfitzger ni-jfitzger mentioned this issue Jun 4, 2023
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ericholscher ericholscher

Labels
Bug A bug
Projects
📍Roadmap
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix status reporting on PRs with the magic exit code readthedocs/readthedocs.org
3 participants
@ericholscher @humitos @benjaoming