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

Use Eloquent-based SubProject model whenever possible #1709

Merged

Conversation

williamjallen
Copy link
Collaborator

@williamjallen williamjallen commented Sep 11, 2023

#1695 created an Eloquent-based SubProject model, but did not attempt to change existing usages of the legacy model. This is a small follow-up PR which expands the usage of the project-subproject relationship to gain access to subprojects through the eloquent-based project model. Amongst other things, this change should make a few pages slightly faster by reducing the number of database queries executed when iterating over subprojects. This PR also unifies the logic for determining which subprojects were active on a given date. It is probable that the prior inconsistent logic caused unexpected behavior in rare cases.

I intend to work on subproject <-> build relationships in a future PR.

Copy link
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our F2F chat, changing the SLOW_PAGE_TIME locally let the subproject test passed successfully. I'm happy with the changes that I can see and manual usage of the page has no issues

Let's see if the CI has an issue with that test when it's in the merge queue.

@josephsnyder josephsnyder added this pull request to the merge queue Sep 13, 2023
Merged via the queue into Kitware:master with commit 5ba108c Sep 13, 2023
2 checks passed
@williamjallen williamjallen deleted the project-subproject-relationship branch September 13, 2023 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants