Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@denis-yuen
Copy link
Member

@denis-yuen denis-yuen commented May 29, 2023

Description
Added github action to cli repo, tested it out with a few alpha releases of the cli (1 through 3)
wiki updated to indicate that the cli and webservice are now basically released the same way without needing individual artifactory accounts https://wiki.oicr.on.ca/display/DOC/Dockstore+Releases#DockstoreReleases-RepositoriesthatuseFriendlyCIversionsandaGitHubAction

This now includes #242 which may be better to review first. I also noticed github actions reusable workflows late, so re-worked to use that to DRY

Review Instructions
Take a look at the wiki instructions, could do another cli release

Issue
https://ucsc-cgl.atlassian.net/browse/SEAB-5516

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 you pass the basic style checks and unit tests by running ./mvnw clean install
  • 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.

@denis-yuen denis-yuen self-assigned this May 29, 2023
@codecov
Copy link

codecov bot commented May 29, 2023

Codecov Report

Patch coverage has no change and project coverage change: -1.49 ⚠️

Comparison is base (4620737) 68.19% compared to head (4d27b5b) 66.71%.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop     #241      +/-   ##
=============================================
- Coverage      68.19%   66.71%   -1.49%     
+ Complexity      1032     1007      -25     
=============================================
  Files             47       47              
  Lines           6081     6081              
  Branches         802      802              
=============================================
- Hits            4147     4057      -90     
- Misses          1581     1690     +109     
+ Partials         353      334      -19     
Flag Coverage Δ
bitbuckettests ?
confidentialtooltests 54.87% <ø> (+5.08%) ⬆️
confidentialworkflowtests ?
nonconfidentialtests 32.28% <ø> (-0.07%) ⬇️
unittests 8.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@denis-yuen denis-yuen changed the title Release/1.15.0 alpha.0 add github actions for release May 29, 2023
@denis-yuen denis-yuen marked this pull request as ready for review May 29, 2023 20:31
@kathy-t
Copy link
Contributor

kathy-t commented May 30, 2023

Was looking at the wiki and noticed that the Dockstore webservice release instructions don't mention creating a GitHub pre-release and attaching the latest CLI dockstore script to the newly drafted webservice release (which are instructions included in the 1.14 staging checklist)

<artifacts>
<artifact>
<file>target/dockstore</file>
<type>.</type>
Copy link
Contributor

@kathy-t kathy-t May 30, 2023

Choose a reason for hiding this comment

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

Was looking on artifactory for this dockstore script and realized it's this file https://artifacts.oicr.on.ca/artifactory/collab-release/io/dockstore/dockstore-client/1.15.0-alpha.3/dockstore-client-1.15.0-alpha.3-dist..

I find the name to be confusing and at first glance at the folder, I couldn't find the dockstore script. We've set the type to be the file name for the openapi and swagger files before here:
https://github.com/dockstore/dockstore/blob/9897d1e08e5281db2966763cd707528a329437f8/dockstore-webservice/pom.xml#L1401

So the file name in artifactory is clearer since it ends in, for example, openapi.yaml. I suggest using the type to set a clearer name for the file, like dockstore

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this was an abortive attempt to use https://stackoverflow.com/questions/27053162/how-to-attach-additional-binary-artifacts-without-type-with-maven-build-helper-m which seems bugged.
I think <type>dockstore</type> will result in dockstore-client-1.15.0-alpha.3-dist.dockstore which, ok.

@denis-yuen
Copy link
Member Author

Was looking at the wiki and noticed that the Dockstore webservice release instructions don't mention creating a GitHub pre-release and attaching the latest CLI dockstore script to the newly drafted webservice release (which are instructions included in the 1.14 staging checklist)

Should be there, search for "For the CLI only" between Step 2 and 3

@kathy-t
Copy link
Contributor

kathy-t commented May 30, 2023

Was looking at the wiki and noticed that the Dockstore webservice release instructions don't mention creating a GitHub pre-release and attaching the latest CLI dockstore script to the newly drafted webservice release (which are instructions included in the 1.14 staging checklist)

Should be there, search for "For the CLI only" between Step 2 and 3

Oh, I meant for the webservice release, not the CLI. The checklist says that the latest CLI dockstore script should be attached to the webservice release

@denis-yuen
Copy link
Member Author

denis-yuen commented May 30, 2023

Oh, I meant for the webservice release, not the CLI. The checklist says that the latest CLI dockstore script should be attached to the webservice release

Hmmm, that seems like a mistake (regression rather) then. The UI quickstart links to the cli repo not the webservice repo https://dockstore.org/quick-start

@denis-yuen denis-yuen requested review from kathy-t and removed request for hyunnaye and kathy-t May 31, 2023 14:10
@@ -0,0 +1,88 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright 2017 OICR
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright year and UCSC

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, I never read these because they're generated. Turns out, whatever plugin we're using copies the copyright notice from the module pom

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@denis-yuen denis-yuen merged commit 1dca21f into develop Jun 1, 2023
@denis-yuen denis-yuen deleted the release/1.15.0-alpha.0 branch June 1, 2023 14:11
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.