feature: Procedural Scheduling
Need I say more?
(will fill out later, just looking at CI tests for now)
Yep, working on them right now using CI tests
@Mythicaeda Ah I didn't push my latest commit with comments / cleanup. Will push when @JoelCourtney is finished force pushing the bug fix
I've gone ahead and rebased this branch to clean up the history. The original history is preserved at https://github.com/NASA-AMMOS/aerie/compare/develop...feature/procedural-scheduling--pre-rebase
This link I think is the best summary for what has changed since @Mythicaeda last reviewed. (based on the timestamps, I think 724263c1a is a good reference point). The biggest difference is I split the one database change into two steps: first add goal invocations, and then add the rest of the procedural scheduling machinery. https://github.com/NASA-AMMOS/aerie/compare/724263c1a..feature/procedural-scheduling
Met with @skovati and @mattdailis and we reviewed all of @Mythicaeda 's remaining unresolved comments.
Those marked with a 👍 reaction are considered in scope and either already fixed, or will be on Monday. Those marked with 👀 will be split off into a new future PR & are out of scope for this release - these are mainly related to fleshing out our e2e tests, and while we all agree there are improvements to be made, it's not worth holding up the release of this feature. We'll work on improved tests starting next week.
We noticed that after our latest round of fix commits, the tests are back to being broken - we'll defer release until Monday & @mattdailis will look at fixing these.
@Mythicaeda let me know if you want to tag up on any specifics
@dandelany I'm specifically concerned with completing a basic test suite being considered "out of scope" for a major feature. The fact that the existing tests have been easy to break via changes this whole PR makes me explicitly concerned that there may be bugs that were added during the review but we're unaware of because the PR was "extensively manually tested" around when it was opened.
I understand that we're trying to get this feature out urgently, which is why I kept the scope of my test requests limited.