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.

Sign up for GitHub

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

Jump to bottom

docs(getting-started): Add CDN to getting started documentation #192

Merged
merged 2 commits into from
Feb 22, 2017
Merged

docs(getting-started): Add CDN to getting started documentation #192

merged 2 commits into from
Feb 22, 2017

Conversation

felipesoares6
Copy link
Contributor

Add about CDN at getting started documentation.

@felipesoares6 felipesoares6 self-assigned this Feb 20, 2017
@codecov-io
Copy link

codecov-io commented Feb 20, 2017 •
edited
Loading

Codecov Report

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

@@          Coverage Diff          @@
##           master   #192   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          20     20           
  Lines         379    379           
=====================================
  Hits          379    379

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 4819485...08bb0ba. Read the comment docs.

vitortalaia
vitortalaia reviewed Feb 20, 2017
View reviewed changes
docs/getting-started.md Outdated
<script scr="https://unpkg.com/garden@0.34.0/dist/js/garden.min.js"></script>
```


Copy link
Contributor

Choose a reason for hiding this comment

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

Uma quebra de linha a mais aqui, não?

@felipesoares6 felipesoares6 force-pushed the docs/add-cdn-to-getting-started branch from adbe8e3 to fb155fb Compare February 20, 2017 19:50
@vitortalaia
Copy link
Contributor

Acho que vale a pena atualizar o README.md também para adicionar essa opção lá

willamesoares
willamesoares reviewed Feb 20, 2017
View reviewed changes
docs/getting-started.md Outdated
@@ -41,5 +61,4 @@ import file from 'node_modules/garden/src/js/file.js';

JavaScript components rely on jQuery to work, most of the time. Please note `garden.min.js` *does not* comes with jQuery. You have to add your own version of it.


##### Checkout the **<a href="https://github.com/leroy-merlin-br/garden" target="_blank" title="Garden Readme">README</a>** of the project in order how to build the project locally.
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems there is a word missing in this line. Maybe the sentence should be: Checkout the ... of the project to know how to build the project locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh thanks for that, I didn't see it.

@willamesoares
Copy link
Contributor

willamesoares commented Feb 20, 2017 •
edited
Loading

Will you adjust the minified file within dist/js in another PR?

fabricionaweb
fabricionaweb reviewed Feb 20, 2017
View reviewed changes
README.md Outdated
JavaScript

```
<script scr="https://unpkg.com/garden@0.34.0/dist/js/garden.min.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Não precisa especificar a versão. Passa @latest ou subtrai que ele pega a ultima

https://unpkg.com/garden/dist/css/garden.min.css
https://unpkg.com/garden/dist/js/garden.min.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just knew about the @latest, that option without the version sounds much better

@felipesoares6
Copy link
Contributor Author

@willamesoares we are having other problems with the minified file, and I'm talking to @leandrogrillo about this.

@felipesoares6 felipesoares6 force-pushed the docs/add-cdn-to-getting-started branch 2 times, most recently from a65f98d to 7ef00ba Compare February 21, 2017 15:14
@felipesoares6 felipesoares6 force-pushed the docs/add-cdn-to-getting-started branch from 7ef00ba to fbff557 Compare February 21, 2017 16:53
@felipesoares6 felipesoares6 force-pushed the docs/add-cdn-to-getting-started branch from fbff557 to 08bb0ba Compare February 22, 2017 19:06
@leandrogrillo leandrogrillo merged commit 08bb0ba into master Feb 22, 2017
@leandrogrillo leandrogrillo deleted the docs/add-cdn-to-getting-started branch February 22, 2017 19:15
@leandrogrillo leandrogrillo added this to the v0.38.0 milestone Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willamesoares willamesoares willamesoares approved these changes

@vitortalaia vitortalaia vitortalaia approved these changes

@leandrogrillo leandrogrillo leandrogrillo approved these changes

@fabricionaweb fabricionaweb fabricionaweb approved these changes

@claudiosikeda claudiosikeda Awaiting requested review from claudiosikeda

Assignees

@felipesoares6 felipesoares6

Labels
should review
Projects
None yet
Milestone
v0.38.0
Development

Successfully merging this pull request may close these issues.
6 participants
@felipesoares6 @codecov-io @vitortalaia @willamesoares @fabricionaweb @leandrogrillo