refactor(app-shell): Migrate protocols to "protocols" directory #13753
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…otocols folder exists Migrates protocols to the protocols directory if protocols_v7.0-supported directory exists.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## chore_release-7.0.1 #13753 +/- ##
=======================================================
- Coverage 71.23% 71.18% -0.06%
=======================================================
Files 2430 2430
Lines 68447 68688 +241
Branches 8043 8145 +102
=======================================================
+ Hits 48759 48894 +135
- Misses 17789 17885 +96
- Partials 1899 1909 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
|
koji
reviewed
Oct 11, 2023
| @@ -344,6 +345,17 @@ export function ProtocolDetails( | |||
| setShowChooseRobotToRunProtocolSlideout(true) | |||
| } | |||
|
|
|||
| const UNKNOWN_ATTACHMENT_ERROR = `${protocolDisplayName} protocol uses | |||
| instruments or modules from a future version of Opentrons software. Please update | |||
There was a problem hiding this comment.
The space between instruments and or is 2-space. Is this on purpose?
If not, ↓
Suggested change
| instruments or modules from a future version of Opentrons software. Please update | |
| instruments or modules from a future version of Opentrons software. Please update |
koji
reviewed
Oct 11, 2023
| @@ -79,13 +79,14 @@ export function ProtocolCard(props: ProtocolCardProps): JSX.Element | null { | |||
| ) | |||
|
|
|||
| const UNKNOWN_ATTACHMENT_ERROR = `${protocolDisplayName} protocol uses | |||
| instruments or modules from a future version of the app. Please update | |||
| instruments or modules from a future version of Opentrons software. Please update | |||
koji
approved these changes
Oct 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes RAUT-716
Overview
This PR is a follow-up to #13561, which addressed pre-parity protocol directory compatibility if a 7.0.0 and <7.0.0 version of the app were used on the same computer. Now that there will only be one supported app soon and all protocols can go in the same directory again, it makes sense to migrate protocols back to the more semantic and familiar directory name, "protocols".
More importantly, there's no need to support a function that checks all protocols in an old directory every time the app is launched - let's migrate all the protocols from the temp 7.0 directory to "protocols" then delete the folder. That way, our only necessary act of atonement moving forward is to check to see if the temp directory exists every startup (asynchronously!).
The one downside to this approach is if two protocols have the same name UUID - the older protocol in the protocol directory will be overwritten. Considering this scenario could only occur for the handful of users utilizing both versions of the app, odds are pretty good.
The error boundary added in the earlier PR will prevent this exact scenario from occurring again!
Test Plan
Changelog
Review requests
Risk assessment
low but long term implications