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

Fix/remove simularium engine specific code #391

Merged
merged 6 commits into from
Jun 5, 2024

Conversation

ascibisz
Copy link
Contributor

Time Estimate or Size

small

Problem

As we fully transition to octopus, it's time to strip out the simularium-engine specific code that is no longer useful
Link to story or ticket

Solution

  • got rid of the --octopus commandline option, as that will now be the assumed platform
  • got rid of the useOctopus option, as that will now be the assumed platform

A website PR will need to be made after this, updating their NetConnectionParams to remove useOctopus

Base automatically changed from fix/one-frame-per-message to main June 3, 2024 21:44
@ascibisz ascibisz marked this pull request as ready for review June 3, 2024 22:03
@ascibisz ascibisz requested a review from a team as a code owner June 3, 2024 22:03
@ascibisz ascibisz requested review from meganrm and ShrimpCryptid and removed request for a team June 3, 2024 22:03
Copy link
Contributor

@ShrimpCryptid ShrimpCryptid left a comment

Choose a reason for hiding this comment

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

Had a few questions but otherwise LGTM!

src/simularium/RemoteSimulator.ts Outdated Show resolved Hide resolved
Comment on lines -15 to -21
const enum PlayBackType {
ID_LIVE_SIMULATION = 0,
ID_PRE_RUN_SIMULATION = 1,
ID_TRAJECTORY_FILE_PLAYBACK = 2,
// insert new values here before LENGTH
LENGTH,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this enum no longer needed? Is it only used by Simularium engine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct! Octopus doesn't use this

Co-authored-by: Peyton Lee <[email protected]>
Copy link

github-actions bot commented Jun 5, 2024

jest coverage report 🧪

Total coverage

Status Category Percentage Covered / Total
🔴 Statements 41.05% 2039/4966
🔴 Branches 43.3% 838/1935
🔴 Functions 37.15% 418/1125
🔴 Lines 41.28% 1952/4728

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

Reduced coverage

Status Filename Statements Branches Functions Lines
🟡 src/simularium 64.36% 59.76% 50.63% 65.32%
🔴 RemoteSimulator.ts 48.71% 60.71% (-1.79% 🔻) 27.41% 52.38% (+1.15% 🔼)
🟢 WebsocketClient.ts 82.66% 69.44% 69.69% 83.56%

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

@ascibisz ascibisz merged commit f219de5 into main Jun 5, 2024
6 checks passed
@ascibisz ascibisz deleted the fix/remove-simularium-engine-specific-code branch June 5, 2024 21:40
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.

3 participants