Skip to content

feat: Run Craft outside of Docker and bundle publish request #4

Merged
BYK merged 5 commits into
mainfrom
byk/build/no-docker
Jan 6, 2021
Merged

feat: Run Craft outside of Docker and bundle publish request #4
BYK merged 5 commits into
mainfrom
byk/build/no-docker

Conversation

@BYK

@BYK BYK commented Dec 22, 2020

Copy link
Copy Markdown
Member

This PR bundles the 'Craft Prepare' and 'Request publish' steps by leveraging the fact that Craft can be run outside of the Docker image, w/o the Craft action as done in getsentry/sentry-javascript#3130. It also improves the automatically crated publish request issue's body by adding the comparison link, normally produced by Craft and tagging the initiator/requester of the release.

This PR SHOULD NOT be merged before pinning all dependents of action-prepare-release as it is very likely to break them.

This action now requires action/checkout to be run beforehand.

@BYK BYK requested review from chadwhitacre and jan-auer December 22, 2020 22:02
This PR bundles the 'Craft Prepare' and 'Request publish' steps by leveraging the fact that Craft can be run outside of the Docker image, w/o the Craft action as done in getsentry/sentry-javascript#3130. It also improves the automatically crated publish request issue's body by adding the comparison link, normally produced by Craft and tagging the initiator/requester of the release.

This PR **SHOULD NOT** be merged before pinning all dependents of
`action-prepare-release` as it is very likely to break them. This action
now requirest `action/checkout` to be run beforehand.

@chadwhitacre chadwhitacre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems reasonable, have not tested. Per commit message need to pin downstreams first to avoid inconsistency.

@BYK BYK force-pushed the byk/build/no-docker branch from 5822d31 to d53d15b Compare December 22, 2020 22:19
@BYK BYK changed the title byk/build/no docker feat: Run Craft outside of Docker and bundle publish request Dec 22, 2020

@jan-auer jan-auer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Optional Suggestion:

Most of our repositories do not use calver, and instead a version should be required. I would add an input to toggle calver that defaults to off:

inputs:
  # ...
  calver:
    description: Creates a CalVer release for the current date if no `version` is specified.
    required: false
    default: false

Comment thread action.yml Outdated
Comment thread action.yml Outdated
Comment thread action.yml Outdated
BYK added a commit to getsentry/sentry that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/snuba that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/relay that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/sentry-javascript that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/self-hosted that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/craft that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/symbolicator that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/sentry-cli that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/sentry that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/snuba that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/self-hosted that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/craft that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/sentry-cli that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/sentry-javascript that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/relay that referenced this pull request Jan 5, 2021
BYK added a commit to getsentry/symbolicator that referenced this pull request Jan 5, 2021
@BYK BYK requested review from chadwhitacre and jan-auer January 5, 2021 21:55

@chadwhitacre chadwhitacre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems reasonable. Did not test. One question about an input description.

Comment thread action.yml
@BYK BYK merged commit a5fd972 into main Jan 6, 2021
@BYK BYK deleted the byk/build/no-docker branch January 6, 2021 10:10
BYK added a commit to getsentry/craft that referenced this pull request Jan 6, 2021
BYK added a commit to getsentry/craft that referenced this pull request Jan 6, 2021
BYK pushed a commit to getsentry/publish that referenced this pull request Jan 7, 2021
In getsentry/action-prepare-release#4, we unintentionally changed the format of the issue title. 

- Before: `publish: <repo>@<version>`
- After: `publish: <owner>/<repo>@<version>`

This is because the `GITHUB_REPOSITORY` env variable includes the full owner and repo name. There is also an `GITHUB_REPOSITORY_OWNER` variable, but none that only contains the repo name. This means we have two options:

1. In the prepare action, remove the owner from the repo. 
2. In the publish action, assume there's an _optional_ owner. 

In this PR, I'm going with option 2, ~~although it is not backwards compatible. This means we will have to change all existing actions before attempting the next release.~~
nEdAy pushed a commit to nEdAy/onpremise that referenced this pull request Jan 13, 2021
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