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

fix: change "Creating Artifacts" to "Uploading Artifacts" #2516

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eseidel
Copy link
Contributor

@eseidel eseidel commented Oct 8, 2024

Customers sometimes get stuck on the "Creating Artifacts" step
which I believe to be due to networking issues. Saying
"Uploading" instead of Creating might help customers understand
that the network is involved in the "creation".

Customers sometimes get stuck on the "Creating Artifacts" step
which I believe to be due to networking issues.  Saying
"Uploading" instead of Creating might help customers understand
that the network is involved in the "creation".
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@@ -412,7 +412,7 @@ Please create a release using "shorebird release" and try again.
required Iterable<Arch> architectures,
String? flavor,
}) async {
final createArtifactProgress = logger.progress('Creating artifacts');
final createArtifactProgress = logger.progress('Uploading artifacts');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final createArtifactProgress = logger.progress('Uploading artifacts');
final uploadArtifactProgress = logger.progress('Uploading artifacts');

can we rename the variable for consistency?

Copy link
Contributor

@felangel felangel Oct 8, 2024

Choose a reason for hiding this comment

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

Actually it might be better to split this into two parts (creating and uploading) for more transparency and improved accuracy? wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In many cases the "creating" is the calling of the API, which implicitly does the "uploading".

@eseidel
Copy link
Contributor Author

eseidel commented Oct 8, 2024

Interesting, Releaser splits into buildReleaseArtifacts and uploadReleaseArtifacts, where as patcher just has buildPatchArtifact and createPatchArtifacts

@@ -511,7 +511,7 @@ aab artifact already exists, continuing...''',
required String extractedAarDir,
required Iterable<Arch> architectures,
}) async {
final createArtifactProgress = logger.progress('Creating artifacts');
final createArtifactProgress = logger.progress('Uploading artifacts');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final createArtifactProgress = logger.progress('Uploading artifacts');
final uploadArtifactProgress = logger.progress('Uploading artifacts');

@@ -606,7 +606,7 @@ aar artifact already exists, continuing...''',
required bool isCodesigned,
required String? podfileLockHash,
}) async {
final createArtifactProgress = logger.progress('Creating artifacts');
final createArtifactProgress = logger.progress('Uploading artifacts');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final createArtifactProgress = logger.progress('Uploading artifacts');
final uploadArtifactProgress = logger.progress('Uploading artifacts');

@@ -658,7 +658,7 @@ aar artifact already exists, continuing...''',
required int releaseId,
required String appFrameworkPath,
}) async {
final createArtifactProgress = logger.progress('Creating artifacts');
final createArtifactProgress = logger.progress('Uploading artifacts');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final createArtifactProgress = logger.progress('Uploading artifacts');
final uploadArtifactProgress = logger.progress('Uploading artifacts');

@@ -138,7 +138,7 @@ class AarPatcher extends Patcher {
);
final patchArtifactBundles = <Arch, PatchArtifactBundle>{};

final createDiffProgress = logger.progress('Creating artifacts');
final createDiffProgress = logger.progress('Uploading artifacts');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final createDiffProgress = logger.progress('Uploading artifacts');
final uploadArtifactsProgress = logger.progress('Uploading artifacts');

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.

2 participants