[eas-cli] add --auto flag to publish #549

jkhales · 2021-08-04T20:35:04Z

Checklist

I've added an entry to CHANGELOG.md if necessary.
I've tagged the changelog entry with [EAS BUILD API] if it's a part of a breaking change to EAS Build API (only possible when updating @expo/eas-build-job package).

Why

We want a flag to automatically set the branch name and update message to the git branch and commit message.

Also made command create a branch if it doesn't already exist.

Test Plan

Tested in demo repo.

linear · 2021-08-04T20:35:06Z

ENG-1490 Automatically create branches on publish if branch does not exist

It is required to run eas branch:create [branch] before running eas branch:publish [branch]. This should create the branch if it's not already there.

The use case is when you're using a CI action to publish, you'd want to be able to publish each time you push a new branch/open a PR, so that you don't have to remember to create the branch before pushing your code to GitHub (for example). This is also nice locally, since when you get your code ready, it's nice to run one command.

Possibly the solution is to create eas publish, where it will auto-create the branch, and perhaps use the current branch if one is specified.

github-actions · 2021-08-04T20:38:19Z

Size Change: +108 B (0%)

Total Size: 37.8 MB

Filename	Size	Change
`./packages/eas-cli/dist/eas-linux-x64.tar.gz`	37.8 MB	+108 B (0%)

_{compressed-size-action}

jkhales · 2021-08-04T21:00:01Z

packages/eas-cli/src/commands/branch/publish.ts

+    case 'Robot': {
+      const { firstName, id } = actor as Pick<Robot, 'firstName' | 'id'>;
+      actorName = firstName ?? `robot: ${id.slice(0, 4)}...`;
+      break;


This is not strictly related to the --auto addition. Noticed we missed a case when fixing up the typing. Curious if this will fix that robot issue @jonsamp was having?

Yeah, this should do it! Thanks for adding this along the way

jkhales · 2021-08-04T21:00:43Z

packages/eas-cli/src/commands/branch/publish.ts

-
-    const { id: branchId, updates } = await viewUpdateBranchAsync({
+    assert(branchName, 'branch name must be specified.');
+    const { id: branchId, updates } = await ensureBranchExists({


No longer throw if branch doesn't exist. Create it instead.

wkozyra95 · 2021-08-05T09:13:25Z

packages/eas-cli/src/commands/branch/publish.ts

@@ -107,13 +137,19 @@ export default class BranchPublish extends Command {
      description: `return a json with the new update group.`,
      default: false,
    }),
+    auto: flags.boolean({


maybe consider sth more descriptive, meaning of the auto in context of publishing is not really obvious

--auto-branch/--auto-message
--managed
or use --non-interactive flag and if values are not provided via --message/--branch us those defaults

Also if you are considering adding updates config to eas.json, options like this would be a good candidate for it

The issue with --non-interactive is that is not clear that this means "apply an opinionated default" instead of "throw if essential args are not provided"

--auto-branch/--auto-message are nice, but maybe too verbose?

Also if you are considering adding updates config to eas.json, options like this would be a good candidate for it

Interesting idea, I'm a bit hesitant to engage there right now as it is in such flux that things tend to get reverted. Maybe once things get more established?

@jonsamp curious what you think about these other flag suggestions.

I agree that --auto-branch and --auto-message are more verbose, and therefore not as developer-friendly. I also suggested --non-interactive or --ci previously, but I agree with JJ that --auto is more correct here since we're applying an opinionated default.

I like --auto. Also, this is pretty low-stakes, since we can always change this flag if our beta users are confused by it.

Thanks for the discussion, will go with --auto for now!

jonsamp

Left some questions and suggestions but this code looks good to me. Excited to use this feature!

jonsamp · 2021-08-06T03:57:08Z

packages/eas-cli/src/commands/branch/publish.ts

@@ -107,13 +137,19 @@ export default class BranchPublish extends Command {
      description: `return a json with the new update group.`,
      default: false,
    }),
+    auto: flags.boolean({


I agree that --auto-branch and --auto-message are more verbose, and therefore not as developer-friendly. I also suggested --non-interactive or --ci previously, but I agree with JJ that --auto is more correct here since we're applying an opinionated default.

I like --auto. Also, this is pretty low-stakes, since we can always change this flag if our beta users are confused by it.

jonsamp · 2021-08-06T04:07:39Z

packages/eas-cli/src/commands/branch/publish.ts

-    if (!name) {
+    if (!branchName && autoFlag) {
+      branchName =
+        (await vcs.getBranchNameAsync()) || `branch-${Math.random().toString(36).substr(2, 4)}`;


We should create another task to make the getBranchNameAsync work inside GitHub Actions. The command this runs is git rev-parse --abbrev-ref HEAD, which works locally, but results in this error during a GitHub Action:

This is not a blocker for this PR, and this should generally not be a problem for CI, since we can pass in the branch name/message programmatically. But it'd be great to run eas branch:publish --auto locally, and then write the same command for the CI action.

.git repository should exist after checkout, if it doesn't then I think it's bug in github action.

Seems like it might be cause by old git version in the image
actions/checkout#335
actions/checkout#126

@jonsamp let me know if @wkozyra95 suggestions for the github action don't work out.

packages/eas-cli/src/commands/branch/publish.ts

jonsamp · 2021-08-06T04:16:02Z

packages/eas-cli/src/commands/branch/publish.ts

@@ -271,6 +311,10 @@ export default class BranchPublish extends Command {
      }
    }

+    if (!message && autoFlag) {
+      message = (await vcs.getLastCommitMessageAsync())?.trim();


Similarly here, this fails when run on CI. It runs git --no-pager log -1 --pretty=%B, but since there's no git repo in an action, we'll see:

Again, no need to update that logic in this PR, but let's make some tasks.

jonsamp · 2021-08-06T04:18:02Z

packages/eas-cli/src/commands/branch/publish.ts

+    case 'Robot': {
+      const { firstName, id } = actor as Pick<Robot, 'firstName' | 'id'>;
+      actorName = firstName ?? `robot: ${id.slice(0, 4)}...`;
+      break;


Yeah, this should do it! Thanks for adding this along the way

packages/eas-cli/src/commands/branch/view.ts

wkozyra95 · 2021-08-06T08:20:01Z

packages/eas-cli/src/commands/branch/publish.ts

-    if (!name) {
+    if (!branchName && autoFlag) {
+      branchName =
+        (await vcs.getBranchNameAsync()) || `branch-${Math.random().toString(36).substr(2, 4)}`;


.git repository should exist after checkout, if it doesn't then I think it's bug in github action.

Seems like it might be cause by old git version in the image
actions/checkout#335
actions/checkout#126

Co-authored-by: Jon Samp <sampjon@gmail.com>

[eas-cli] add --auto flag to publish

5a4d004

add change log

bf3d36d

tweaks

c8c3356

jkhales commented Aug 4, 2021

View reviewed changes

jkhales requested review from wkozyra95 and jonsamp August 4, 2021 21:01

jkhales marked this pull request as ready for review August 4, 2021 21:01

jkhales removed the request for review from wkozyra95 August 4, 2021 21:02

wkozyra95 reviewed Aug 5, 2021

View reviewed changes

jonsamp approved these changes Aug 6, 2021

View reviewed changes

wkozyra95 approved these changes Aug 6, 2021

View reviewed changes

jkh and others added 2 commits August 6, 2021 08:52

Update packages/eas-cli/src/commands/branch/view.ts

f439898

Co-authored-by: Jon Samp <sampjon@gmail.com>

move assert inside

1308b3b

jkhales merged commit ece93da into main Aug 6, 2021

jkhales deleted the jonathan/eng-1490-automatically-create-branches-on-publish branch August 6, 2021 12:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[eas-cli] add --auto flag to publish #549

[eas-cli] add --auto flag to publish #549

jkhales commented Aug 4, 2021 •

edited

Loading

linear bot commented Aug 4, 2021

github-actions bot commented Aug 4, 2021 •

edited

Loading

jkhales Aug 4, 2021

jonsamp Aug 6, 2021

jkhales Aug 4, 2021

wkozyra95 Aug 5, 2021

jkhales Aug 5, 2021

jonsamp Aug 6, 2021

jkhales Aug 6, 2021

jonsamp left a comment

jonsamp Aug 6, 2021

jonsamp Aug 6, 2021

wkozyra95 Aug 6, 2021

jkhales Aug 6, 2021

jonsamp Aug 6, 2021

jonsamp Aug 6, 2021

wkozyra95 Aug 6, 2021

[eas-cli] add --auto flag to publish #549

[eas-cli] add --auto flag to publish #549

Conversation

jkhales commented Aug 4, 2021 • edited Loading

Checklist

Why

Test Plan

linear bot commented Aug 4, 2021

github-actions bot commented Aug 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonsamp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkhales commented Aug 4, 2021 •

edited

Loading

github-actions bot commented Aug 4, 2021 •

edited

Loading