Skip to content
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

Reorder presentation of batches #658

Merged
merged 3 commits into from Jun 8, 2019

Conversation

Projects
None yet
2 participants
@umamaistempo
Copy link
Contributor

commented Jun 7, 2019

Based on #639

Rationale: For someone administering a bors instance, usually it should be more important to see the build status instead of what still needs to be reviewed (as this would usually be done by checking the github notifications). And considering :running | :waiting is a short transitive state, the list of such batches is way shorter and will disappear quickly, so not affecting the use case of people that uses the web view to see what needs reviewing.

Note: also added an order to other queries on the batch model as to ensure consistency (ie: without an explicit ordering on postgres - and other dbs -, there's no order guarantee, so results are somewhat non-deterministic)

@umamaistempo umamaistempo force-pushed the umamaistempo:i-639-reorder-batches-on-web branch from 7057a3d to f284383 Jun 7, 2019

Enum.each(batches, &send_status(repo_conn, &1, :delayed))
Batch
|> where([b], b.id in ^ids)
|> Repo.update_all(set: [state: :waiting])

This comment has been minimized.

Copy link
@umamaistempo

umamaistempo Jun 7, 2019

Author Contributor

I practice, the changes to this function, if affecting DB performance, will improve it, because we were going to fetch the batches before, and by using the ids directly instead of the more complex query for join might benefit the query planner (unless it's properly caching and identifying that the resultset would be the same, in such case there's virtually no difference between both).

So, this change does not degrade performance nor readability :>

This comment has been minimized.

Copy link
@notriddle

notriddle Jun 7, 2019

Member

Agreed.

@notriddle
Copy link
Member

left a comment

bors r+

bors bot added a commit that referenced this pull request Jun 7, 2019

Merge #658
658: Reorder presentation of batches r=notriddle a=umamaistempo

Based on #639 

Rationale: For someone administering a bors instance, usually it should be more important to see the build status instead of what still needs to be reviewed (as this would usually be done by checking the github notifications). And considering `:running | :waiting` is a short transitive state, the list of such batches is way shorter and will disappear quickly, so not affecting the use case of people that uses the web view to see what needs reviewing.

Note: also added an order to other queries on the batch model as to ensure consistency (ie: without an explicit ordering on postgres - and other dbs -, there's no order guarantee, so results are somewhat non-deterministic)

Co-authored-by: Charlotte Lorelei de Oliveira <11341349+umamaistempo@users.noreply.github.com>
@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2019

Build succeeded

@bors bors bot merged commit 595c728 into bors-ng:master Jun 8, 2019

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
bors Build succeeded
Details

notriddle added a commit to bors-ng/bors-ng.github.io that referenced this pull request Jul 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.