...
Chris Colvard
Notetaker:
Thomas Scherz
Meeting was adjourned at 09:11EST/12:11 EST
Hyrax Pull Request Reviews
Attendees
Pull Request 3548
- Chromedriver was failing, this PR installs Google Chrome on CircleCI build containers in order to ensure that Chrome is always stable
- It does ensure that we remain dependent upon Debian package releases for Chrome stable
- It was merged
Pull Request 3541
- Draft PR (new feature introduced by GitHub)
- Merge icon is grey, otherwise not easily distinguishable from normal PRs
- Could apply "in progress" label, but this would be unnecessary
- Should we review them? Or wait for them to become full PRs?
- This already had a review
- "in progress" labels would only be useful if Waffle is used for tracking issues
- Chris commented requesting a rebase
Backport PRs for CircleCI
Pull Request 3546
- Transaction becomes a no-op actor
- Already merged into master, deals with the issue where if you hit an exception in the actor stack, it renders the system unusable
- IDs would be rolled back (NOIDs), but Fedora wouldn't roll back its own resource
- NOID would be minted using a Fedora ID for a Fedora resource which already existed
- This would result in a LDP Conflict
- This PR ensures that the conflicts are handled and resolved
- Tom Johnson will handle the CI build issues
Pull Request 3543
- Alerts users of data loss when moving between tabs while editing Collection metadata
- No tests are present in the PR
- This behavior may already be there for certain cases, but not for all cases
- Tom Johnson commented only 15 hours before this call
Pull Request 3545
- This is another backport
- This should require a rebase
Pull Request 3544
- (This is the same case as 3545)
Pull Request 3468
- This has not had any activity since last week
- It was marked as stale, Chris and Tom discussed this last week
- Tom just needs to come back and introduce additional commits
Pull Request 3489
- This has had no activity since last week
- Invite someone from the analytics sprint to perform a review
Pull Request 3472
- This is going to be abandoned in favor of a PR being introduced by Tom Johnson
Pull Request 3431
- Chris and Tom discussed this last week
- Backports support for dealing with transactional request in Noid/Fedora conflict resolution (addressing the same problem as 3546)
- Lynette Rayle provided a different solution, which set up validation for identifiers
- Having Noid::Rails check to see if the ID is already in use, and then know to generate a new one
- It does work and solves a larger problem
- However, there are concerns about it introducing performance issues
- Noid ID minting calls out to Fedora, potentially twice (to determine if it exists, two HEAD requests are issued)
- Unnecessary overhead...and it is dealt with in backports
- Tom suggested that this just be closed
- Perhaps place this in the management guide rather than just the README
- "If you're running into problems, and you need strict validation, you can follow this pattern"
- Chris commented on the PR
3504
- Holdover from last Hyrax/Valkyrie sprint, still WIP
Review concluded at 09:36PST/12:36EST