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

Run on first page load and ensure conversion review has proper behavior #768

Merged
merged 10 commits into from
May 3, 2024

Conversation

garrettmflynn
Copy link
Member

When going through a straight run of the SpikeGLX-Phy test pipeline, it was noticed that full and preview conversions were not being triggered.

This PR fixes this behavior to ensure that a preview conversion is triggered on the first run even if no manual changes have been made by the user. The behavior of the Conversion Review page has also been updated to trigger a conversion before reaching DANDI Upload.

@garrettmflynn garrettmflynn self-assigned this May 2, 2024
@CodyCBakerPhD
Copy link
Collaborator

Any way to add tests? (also some failing atm)

@garrettmflynn
Copy link
Member Author

Gotcha thanks for flagging! Added a check to the tests so that main would fail and fixed how tests are handled (specifically, so that transitions are awaited properly before moving forward with the next test) so that these succeed

@CodyCBakerPhD CodyCBakerPhD enabled auto-merge May 2, 2024 21:06
@CodyCBakerPhD
Copy link
Collaborator

@garrettmflynn 2 pages in the storybook now fail to fetch - is this intended or do you know what's happening?

@garrettmflynn
Copy link
Member Author

Found the issue. Fixed

@CodyCBakerPhD
Copy link
Collaborator

@garrettmflynn Main tests failing again


const { desyncedData } = info.globalState;

f
Copy link
Member Author

Choose a reason for hiding this comment

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

Here's the culprit. Whoops!

@garrettmflynn
Copy link
Member Author

garrettmflynn commented May 3, 2024

Looks like one of my wonderful cats stepped on the keyboard and inserted a stray f in the script.

My bad! Works locally now

@CodyCBakerPhD
Copy link
Collaborator

Looks like one of my wonderful cats stepped on the keyboard and inserted a stray f in the script.

😂

@CodyCBakerPhD CodyCBakerPhD merged commit c501319 into main May 3, 2024
16 of 17 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the fix-sync-state-tracking branch May 3, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants