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

refactor(update): remove republish from update command #1535

Merged
merged 13 commits into from
Dec 5, 2022

Conversation

byCedric
Copy link
Member

@byCedric byCedric commented Nov 21, 2022

Checklist

  • I've added an entry to CHANGELOG.md if necessary. You can comment this pull request with /changelog-entry [breaking-change|new-feature|bug-fix|chore] [message] and CHANGELOG.md will be updated automatically.

Why

Follow up of #1533, part of ENG-5926

How

  • Kept the --group and --republish flags in the command
  • Added warning with new command when using flags
  • Dropped all related code to republish from the update command

Test Plan

When using --group or --republish, users should now get a deprecation warning with the new command. This replacement command uses the --branch and --group flags, to help users just copy whatever they need.

image

@github-actions
Copy link

github-actions bot commented Nov 21, 2022

Size Change: +604 B (0%)

Total Size: 40.3 MB

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

compressed-size-action

@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Merging #1535 (6626f37) into main (33f4ae7) will increase coverage by 0.26%.
The diff coverage is 50.32%.

❗ Current head 6626f37 differs from pull request most recent head e00bd3e. Consider uploading reports for the commit e00bd3e to get more accurate results

@@            Coverage Diff             @@
##             main    #1535      +/-   ##
==========================================
+ Coverage   51.13%   51.38%   +0.26%     
==========================================
  Files         461      460       -1     
  Lines       16066    15973      -93     
  Branches     3175     3141      -34     
==========================================
- Hits         8213     8206       -7     
+ Misses       7838     7752      -86     
  Partials       15       15              
Impacted Files Coverage Δ
packages/eas-cli/src/commands/update/index.ts 16.83% <0.00%> (+2.16%) ⬆️
packages/eas-cli/src/graphql/generated.ts 100.00% <ø> (ø)
...ckages/eas-cli/src/graphql/queries/PublishQuery.ts 23.08% <0.00%> (-10.25%) ⬇️
packages/eas-cli/src/graphql/types/Update.ts 100.00% <ø> (ø)
packages/eas-cli/src/commands/update/republish.ts 74.08% <74.08%> (ø)
packages/eas-cli/src/commands/build/run.ts 22.67% <0.00%> (-1.42%) ⬇️
packages/eas-cli/src/build/runBuildAndSubmit.ts 24.82% <0.00%> (-0.33%) ⬇️
packages/eas-cli/src/devices/utils/formatDevice.ts 68.19% <0.00%> (ø)
packages/eas-cli/src/run/utils.ts
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@byCedric byCedric marked this pull request as ready for review November 23, 2022 14:04
@@ -112,11 +109,11 @@ export default class UpdatePublish extends EasCommand {
required: false,
}),
republish: Flags.boolean({
description: 'Republish an update group',
description: 'Republish an update group (deprecated, see republish command)',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description: 'Republish an update group (deprecated, see republish command)',
description: 'Republish an update group',
deprecated: true,

(or feel free to leave stuff in description and add deprecated field)

Copy link
Member Author

@byCedric byCedric Nov 25, 2022

Choose a reason for hiding this comment

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

The deprecated feature from OCLIF is far from optimal. It produces a warning on every command invocation, even when not using the flag. Not a strong opinion, but I would advice against using the deprecated feature.

See aborted PR #1500 for an example

exclusive: ['input-dir', 'skip-bundler'],
}),
group: Flags.string({
description: 'Update group to republish',
description: 'Update group to republish (deprecated, see republish command)',
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

packages/eas-cli/src/commands/update/index.ts Outdated Show resolved Hide resolved
packages/eas-cli/src/commands/update/index.ts Outdated Show resolved Hide resolved
packages/eas-cli/src/commands/update/index.ts Outdated Show resolved Hide resolved
@byCedric byCedric force-pushed the @bycedric/update/clean-up-republish branch from bf0517f to deb311d Compare November 25, 2022 14:24
@byCedric
Copy link
Member Author

/changelog-entry chore Remove old republish code from the update command

CHANGELOG.md Outdated
@@ -12,6 +12,8 @@ This is the log of notable changes to EAS CLI and related packages.

### 🧹 Chores

- Remove old republish code from the update command. ([#1535](https://github.com/expo/eas-cli/pull/1535) by [@byCedric](https://github.com/byCedric))
Copy link
Contributor

Choose a reason for hiding this comment

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

Rebase and move to the correct group.

Copy link
Member Author

Choose a reason for hiding this comment

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

I reverted, rebased, merged conflicts, and recreated changelog entries for both #1533 and #1535.

@byCedric byCedric force-pushed the @bycedric/update/split-republish branch from 0c7dcf4 to 5de5a9a Compare November 25, 2022 14:46
@byCedric byCedric force-pushed the @bycedric/update/clean-up-republish branch from 08ece28 to 6626f37 Compare November 25, 2022 14:59
@byCedric
Copy link
Member Author

/changelog-entry chore Remove old republish code from the update command

@byCedric byCedric force-pushed the @bycedric/update/clean-up-republish branch from c33c9b4 to 8c48bfd Compare November 25, 2022 15:08
@wkozyra95 wkozyra95 removed their request for review November 29, 2022 11:46
Base automatically changed from @bycedric/update/split-republish to main December 5, 2022 12:08
@byCedric byCedric force-pushed the @bycedric/update/clean-up-republish branch from 8c48bfd to e00bd3e Compare December 5, 2022 12:20
@byCedric byCedric merged commit 4e11707 into main Dec 5, 2022
@byCedric byCedric deleted the @bycedric/update/clean-up-republish branch December 5, 2022 12:25
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