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

Update documentation of the section plugin Google Cloud Storage #6275

Merged
merged 2 commits into from Jun 5, 2020

Conversation

Lith
Copy link
Contributor

@Lith Lith commented May 19, 2020

Hello,

Some people are writing issue about misunderstood in the google cloud storage section for the deployment with Google App Engine.
In order to avoid further mistake, I prefer than the documentation provide a redirection to the full documentation of the plugin.

Thanks,

@derrickmehaffy
Copy link
Member

While I do like gutting this out, @lauriejim we need to clarify if we are going to be directly linking community plugins in the official documentation.

(Or at the very least we should probably make a notice that this plugin is not developed by Strapi)

@derrickmehaffy derrickmehaffy self-requested a review May 19, 2020 13:21
derrickmehaffy
derrickmehaffy previously approved these changes May 19, 2020
Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

This LGTM, but need Jim to approve and/or maybe add a notice about it not being managed by Strapi.

@Lith
Copy link
Contributor Author

Lith commented May 19, 2020

Hi @derrickmehaffy , thanks for your fast reply.

I've wrote from the beginning on the documentation of the plugin, it's a "Non-Official" Google Cloud Storage Provider for Strapi Upload.
But I can add additional information in order to inform it's not managed by Strapi Team.

The main problem for now, it's some user send me help request for Google App Engine and how to use the plugin with deprecated information.

Thanks :)

@derrickmehaffy
Copy link
Member

Ah yeah perfect, I hadn't reviewed the other part of that documentation. I am certainly not apposed to having community packages in the docs. :)

@lisongx
Copy link

lisongx commented May 19, 2020

@Lith
thanks for doing this! was literally trying to find the non-existing button for 20 minutes until I go to the documentation for the plugin.

@codecov
Copy link

codecov bot commented May 19, 2020

Codecov Report

Merging #6275 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6275   +/-   ##
=======================================
  Coverage   19.85%   19.85%           
=======================================
  Files         857      857           
  Lines       12076    12076           
  Branches     1963     1963           
=======================================
  Hits         2398     2398           
  Misses       8093     8093           
  Partials     1585     1585           
Flag Coverage Δ
#front 14.66% <ø> (ø)
#unit 41.26% <ø> (ø)
Impacted Files Coverage Δ
...ntent-type-builder/controllers/validation/types.js 52.27% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 759f09d...a94dffa. Read the comment docs.

@Lith
Copy link
Contributor Author

Lith commented May 25, 2020

Hi, I've fixed conflict and rebase to latest master commit.

derrickmehaffy
derrickmehaffy previously approved these changes May 26, 2020
Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

LGTM, I would prefer directly linking to the source as it's easier for @Lith to maintain as apposed to updating the documentation.

@Lith
Copy link
Contributor Author

Lith commented May 29, 2020

Travis are failing with MySQL. Does it possible to check and approve this PR ?
Thanks :)

@derrickmehaffy
Copy link
Member

Travis are failing with MySQL. Does it possible to check and approve this PR ?
Thanks :)

You can ignore that, since you are modifying the docs. It has to do with the knex pooling....

lauriejim
lauriejim previously approved these changes Jun 2, 2020
Copy link
Contributor

@lauriejim lauriejim left a comment

Choose a reason for hiding this comment

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

LGTM

@lauriejim
Copy link
Contributor

Hello @Lith can you please also apply this update in the v3.x documentation?

@Lith Lith dismissed stale reviews from lauriejim and derrickmehaffy via 931647a June 2, 2020 14:49
Signed-off-by: Vanessa Pasque (Lith) <lith@amykron.com>
@Lith
Copy link
Contributor Author

Lith commented Jun 2, 2020

Hi @lauriejim !
I've apply modification on v3.x documentation
Let me know if something wrong :)
Thanks <3

Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

Perfect thank you @Lith

@lauriejim lauriejim added source: docs Documentation changes issue: enhancement Issue suggesting an enhancement to an existing feature labels Jun 5, 2020
@lauriejim lauriejim added this to the 3.0.2 milestone Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: enhancement Issue suggesting an enhancement to an existing feature source: docs Documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants