Skip to content

Conversation

@Ehesp
Copy link
Member

@Ehesp Ehesp commented Jan 31, 2020

  • Fix: Files are not always uploaded with a default content type (e.g. gsutils).
  • Docs: File validation is unknown to the user. Added a note which indicates a file must have a content type, and one which starts with the image/ prefix.

Fixes #175

@googlebot googlebot added the cla: yes Author has signed the CLA label Jan 31, 2020
@Ehesp Ehesp added do not merge Do not merge this Pull Request and removed cla: yes Author has signed the CLA labels Jan 31, 2020
@googlebot
Copy link

☹️ Sorry, but only Googlers may change the label cla: yes.

@googlebot googlebot added the cla: yes Author has signed the CLA label Jan 31, 2020
@rachelsaunders
Copy link
Contributor

With what GitHub Issue is this PR associated? Could it please be added to the PR description?

@Ehesp
Copy link
Member Author

Ehesp commented Feb 1, 2020

@rachelsaunders added it in, cross linked it from the issue rather than the PR.

@karayu karayu added the in-progress A fix or resolution is in progress label Feb 3, 2020
@laurenzlong laurenzlong self-requested a review February 5, 2020 21:32
1. Go to your [Storage dashboard](https://console.firebase.google.com/project/${param:PROJECT_ID}/storage).

1. Upload an image file to the bucket: `${param:IMG_BUCKET}`
1. Upload an image file to the bucket: `${param:IMG_BUCKET}`. The file must have a valid [image MIME Type](https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types#Image_types), for example `image/png`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a really common case that a dev might not have the image MIME type specified? If not, then I wouldn't put this level of detail here in this uber quickstart section. Instead, I'd like to explore putting some of these known gotchas within the "Using this extension" section.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, agreed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Moved this "error" down in the POSTINSTALL file. Pls double-check that my change and additional text is correct? Thx!

Copy link
Contributor

Choose a reason for hiding this comment

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

ping on this comment thread :-)

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

@laurenzlong Are you ok with the changes?

…be specified

This "error" isn't a common error, so a warning about it doesn't really belong in the uber quickstart section.  But, it seems that we're collecting a few "be aware of this or that" items, so it's better if we collect these into a specific section of the instructions doc.
@agau4779 agau4779 merged commit a2bf4a9 into next Mar 3, 2020
@agau4779 agau4779 deleted the @invertase/resize-content-type branch March 3, 2020 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Author has signed the CLA do not merge Do not merge this Pull Request in-progress A fix or resolution is in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

image cannot read property startswith of undefined

7 participants