| bsnodgrass (he/him) | Greetings from St Charles IL, far western burbs of Chicago - cool and supposed to rain through midday, but dry so far |
| fjgarlin | Hi there from Cadiz, warm and sunny on this side of the planet - fjgarlin (edited) |
1 I'm ok with people making fairly liberal use of the Project Browser MVP tag if you think it's something we cannot or should not ship a 1.0 without; that fills the contribkanban which is useful to us. So, permission granted!
| chrisfromredfin | I will wait for tests then merge this. |
| chrisfromredfin | merged. |
:
| bnjmnm | I can review. I do have a question about the issue in general: When I did the PB hackathon it seemed like the release data was being used (or there were plans to) . Does anyone happen to know if this was descoped permanently or just for the MVP. I just want to make sure we're not ripping something out that will need to get put back later |
| fjgarlin | the reason for it is because, right now, it’s not used anywhere, so it’s adding complexity for the MVP unnecessarily, so we were in favor of “cleaning up” if it’s not used at all |
| chrisfromredfin | The release data that we're interested in will be computed and handed over with the project, no need to query releases individually. Right @fjgarlin? |
| fjgarlin | in the future, when we have better idea of how/when/why use it, we can add it back, but not necessarily in that same format |
| chrisfromredfin | The #1 reason for needing to query releases is to get usage data; and I feel like we can hand that back in the main API call for the project. |
| fjgarlin | correct. core compatibility and usage will be now present at project level, so no need for releases |
| fjgarlin | the mock does not use releases for usage, and the jsonapi endpont will not use releases for usage. all that data will come with the project’s data |
| chrisfromredfin | (Also, my hot take on "don't get rid of code we might use later" is "that's what Git is for" :wink: ) |
| fjgarlin | if we merge the fixture fix first. I’ll need to rebase this one and change a small thing. |
| chrisfromredfin | OK. I'm going to do one final review of the fixture builder, then close my eyes and go - cuz it'll be good to get done. And I can always revert it. |
| chrisfromredfin | And tests are passing, so. |
| fjgarlin | (as soon as you do it, I’ll do the changes needed here. no rush tho) |
| chrisfromredfin | ok, re-reviewed the code. then fired it up in DrupalPod. Then uninstalled and reinstalled and checked again. Regardless, it doesn't seem to break anything. |
| chrisfromredfin | :crossed_fingers: |
| chrisfromredfin | Merged! |
| chrisfromredfin | Spinning up 1.0.x HEAD on DrupalPod now as a sanity check :stuck_out_tongue: |
| fjgarlin | #3283825: Releases endpoint not really needed. Clean up unused code. rebased. The code looks good to me. Will need testing. |
| fjgarlin | fixture regeneration |
| fjgarlin | #3280006: May 2022 - Update Fixture |
| chrisfromredfin | ok so seems like we still need this in MockDrupalApi to make pagination work, yeah? |
| fjgarlin | guess so, for the sorting. but that code is in the fixture regeneration. so we should merge that in first, and then see if #3282648 is needed at all or not |
| chrisfromredfin | Ahh yes, ok. |
| fjgarlin | see the code here: https://git.drupalcode.org/project/project_browser/-/merge_requests/137/... |
| chrisfromredfin | Can you rebase 3280006 and I can put that in my queue? |
| fjgarlin | sure |
| fjgarlin | gotta love clean rebases, done :slightly_smiling_face: |
| chrisfromredfin | woo woo |
| chrisfromredfin | So spinning up the 1.0.x head on DrupalPod, and then navigating to the last page, I see "No records available" which seems odd. Perhaps we do need the fix in MockDrupalAPI also? |
Participants:
chrisfromredfin, bnjmnm, fjgarlin, leslieg
Comments
Comment #5
leslieg commentedComment #6
leslieg commented