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

[eas-cli][eas-update] Allow undefined update message #2148

Merged
merged 2 commits into from
Jan 4, 2024

Conversation

wschurman
Copy link
Member

@wschurman wschurman commented Dec 13, 2023

Why

This was requested for cases where there isn't a VCS and also no update message is manually supplied.

Closes ENG-9633.

How

It is already optional in the GraphQL mutation, so this is just a product decision that we need to deliberately make.

For Reviewers

I think it's worth having a discussion on the UX benefits/costs of this product decision. This PR is meant to just start the discussion, not convey a strong intent to make the change as I'm probably oblivious to potential downsides.

Test Plan

Create an update and don't specify message.

Copy link

linear bot commented Dec 13, 2023

@wschurman wschurman requested review from quinlanj and removed request for khamilowicz, EvanBacon and szdziedzic December 13, 2023 03:41
@wschurman
Copy link
Member Author

/changelog-entry new-feature Allow undefined update message for EAS Update publishing

Copy link

github-actions bot commented Dec 13, 2023

Size Change: -488 B (0%)

Total Size: 55.5 MB

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

compressed-size-action

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (2dab784) 54.22% compared to head (21d0fa7) 54.21%.

❗ Current head 21d0fa7 differs from pull request most recent head d81d4c8. Consider uploading reports for the commit d81d4c8 to get more accurate results

Files Patch % Lines
packages/eas-cli/src/project/publish.ts 12.50% 7 Missing ⚠️
packages/eas-cli/src/vcs/clients/git.ts 0.00% 2 Missing ⚠️
packages/eas-cli/src/vcs/clients/noVcs.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2148      +/-   ##
==========================================
- Coverage   54.22%   54.21%   -0.00%     
==========================================
  Files         509      509              
  Lines       18663    18670       +7     
  Branches     3737     3741       +4     
==========================================
+ Hits        10119    10121       +2     
- Misses       8523     8528       +5     
  Partials       21       21              

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

@quinlanj
Copy link
Member

Most of our UI's and tooling rely on people to provide a message to be usable, and we assume most people are using some kind of VCS. No VCS would be a bad user experience, but in the absence of a VCS, we should not be blocking people from publishing.

@wschurman
Copy link
Member Author

That's a good point. Maybe we should only allow this to be empty when there's no VCS?

@wschurman wschurman force-pushed the @wschurman/allow-blank-update-message branch from 952e590 to 21d0fa7 Compare January 4, 2024 20:05
@wschurman
Copy link
Member Author

/changelog-entry new-feature Allow undefined update message for EAS Update publishing when no VCS

Copy link

github-actions bot commented Jan 4, 2024

✅ Thank you for adding the changelog entry!

@wschurman wschurman merged commit eeb143a into main Jan 4, 2024
6 checks passed
@wschurman wschurman deleted the @wschurman/allow-blank-update-message branch January 4, 2024 20:10
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.

None yet

2 participants