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

Update Mac OS Quickstart #1964

Merged
merged 5 commits into from
May 1, 2024
Merged

Conversation

coverbeck
Copy link
Collaborator

@coverbeck coverbeck commented Apr 12, 2024

Description
There are lots of ways to install Java on a Mac, so be less prescriptive than we have been. The other changes are from PR #1843, which I left unchanged, and were approved in that PR. The Mac Java install changes are based from my comments on that PR.

Before

Screen Shot 2024-04-16 at 1 54 38 PM

After
Screen Shot 2024-04-16 at 1 54 13 PM

Review Instructions
Go to quick start, from the My Dockstore page, and verify the Mac OS instructions look good.

Issue
SEAB-6372

Security
If there are any concerns that require extra attention from the security team, highlight them here.

Please make sure that you've checked the following before submitting your pull request. Thanks!

  • Check that your code compiles by running npm run build
  • Ensure that the PR targets the correct branch. Check the milestone or fix version of the ticket.
  • If this is the first time you're submitting a PR or even if you just need a refresher, consider reviewing our style guide
  • Do not bypass Angular sanitization (bypassSecurityTrustHtml, etc.), or justify why you need to do so
  • If displaying markdown, use the markdown-wrapper component, which does extra sanitization
  • Do not use cookies, although this may change in the future
  • Run npm audit and ensure you are not introducing new vulnerabilities
  • Do due diligence on new 3rd party libraries, checking for CVEs
  • Don't allow user-uploaded images to be served from the Dockstore domain
  • If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.
  • Check whether this PR disables tests. If it legitimately needs to disable a test, create a new ticket to re-enable it in a specific milestone.

I tested the `brew` instructions on a fresh laptop that does not use Apple Silicon, but does use zsh. Docker 4.3.0 was chosen since that's the first version to support Apple Silicon.
Java and Docker dependencies are each now their own "part", and the two ways to install openJDK 17 have subheadings -- the combination of these two changes makes the Mac OS Java installation parts much less confusing.

Additional zsh notice made on adding dockstore CLI to the path.

Tell people their version of openJDK will vary.

Give a few use cases for the Dockstore CLI after installing the required bits, and clarify WDL doesn't require any additional downloads.

Remove some unnecessary numbering.
@coverbeck coverbeck self-assigned this Apr 12, 2024
@coverbeck coverbeck mentioned this pull request Apr 12, 2024
11 tasks
Copy link

codecov bot commented Apr 12, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 40.32%. Comparing base (d8aba8a) to head (51fb016).

Files Patch % Lines
...g/downloadcliclient/downloadcliclient.component.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1964   +/-   ##
========================================
  Coverage    40.32%   40.32%           
========================================
  Files          400      400           
  Lines        11957    11957           
  Branches      2894     2894           
========================================
  Hits          4822     4822           
  Misses        4846     4846           
  Partials      2289     2289           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coverbeck
Copy link
Collaborator Author

If this is approved by all, please merge while I'm out.

@denis-yuen
Copy link
Member

(adding a few more mac users)

Copy link
Member

@denis-yuen denis-yuen left a comment

Choose a reason for hiding this comment

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

Superficially ok, but I run on Ubuntu so can't judge much of this beyond syntax and clarity

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@coverbeck coverbeck merged commit 98d079a into develop May 1, 2024
17 of 19 checks passed
@coverbeck coverbeck deleted the feature/update-mac-os-quickstart branch May 1, 2024 18:25
coverbeck pushed a commit that referenced this pull request Jul 19, 2024
See PR comments.

(cherry picked from commit 98d079a)
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.

6 participants