Skip to content

Conversation

@flovilmart
Copy link
Contributor

@flovilmart flovilmart commented Nov 27, 2017

This PR fixes a few issues when generating docs with JS docs, See the generated docs here:

https://flovilmart.github.io/parse-js-docs-demo/index.html

If we're all ok with it, I'll push to the gh-pages branch and automate the release on tags.

@codecov
Copy link

codecov bot commented Nov 27, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #512   +/-   ##
=======================================
  Coverage   84.03%   84.03%           
=======================================
  Files          47       47           
  Lines        3801     3801           
  Branches      869      869           
=======================================
  Hits         3194     3194           
  Misses        607      607
Impacted Files Coverage Δ
src/Push.js 100% <ø> (ø) ⬆️
src/LiveQueryClient.js 86.95% <ø> (ø) ⬆️
src/ParseError.js 100% <ø> (ø) ⬆️
src/ParseSession.js 89.74% <ø> (ø) ⬆️
src/Cloud.js 96% <ø> (ø) ⬆️
src/ParseRole.js 80% <ø> (ø) ⬆️
src/escape.js 100% <ø> (ø) ⬆️
src/ParseGeoPoint.js 89.7% <ø> (ø) ⬆️
src/CoreManager.js 100% <ø> (ø) ⬆️
src/ParseLiveQuery.js 80% <ø> (ø) ⬆️
... and 13 more

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 942305a...e6ce9d3. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Nov 27, 2017

@flovilmart
Copy link
Contributor Author

@dplewis yeah I've noticed that as well, I'm not sure what's causing it, and why?

Copy link
Contributor

@acinader acinader left a comment

Choose a reason for hiding this comment

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

super

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

Much needed!

@flovilmart
Copy link
Contributor Author

I need to find out why the docs are duplicated...

@flovilmart
Copy link
Contributor Author

Other question/ more management wise. Do we keep the api references in each gh-pages or do we move them on the docs repo?

@dplewis
Copy link
Member

dplewis commented Nov 28, 2017

I tried it locally earlier and ran npm run docs and got a different result than https://flovilmart.github.io/parse-js-docs-demo/index.html

When I removed includePattern everything looks ok.

@flovilmart
Copy link
Contributor Author

@dplewis still is broken, some docs are in double still :/

@dplewis
Copy link
Member

dplewis commented Nov 28, 2017

@flovilmart I prefer the gh-pages. Looking at the php sdk, gh-pages is far ahead of php docs.

@flovilmart
Copy link
Contributor Author

@dplewis it seems that removing those configs did the trick, check again the demo repo, all look good now, some private (_) methods are still missing some docs, but we can live with it.

@dplewis
Copy link
Member

dplewis commented Nov 28, 2017

@flovilmart jsdoc is weird. includes > includePattern. But if the includes doesn't match the includePattern it gets ignored or duplicated.

@flovilmart
Copy link
Contributor Author

Yeah I figured the hard way, now the docs look good though!
I'll open a PR against gh-pages.;

@flovilmart
Copy link
Contributor Author

@dplewis
Copy link
Member

dplewis commented Nov 28, 2017

lgtm!

@flovilmart
Copy link
Contributor Author

I need to automate that on release at one point... May not be trivial but we'll see.

@flovilmart
Copy link
Contributor Author

we could use that: https://docs.travis-ci.com/user/deployment/pages/
But because of the nature of gh-pages deployment / force push, this would override any existing content (unless cloned beforehand)

@dplewis
Copy link
Member

dplewis commented Nov 28, 2017

@montymxb advice on gh-pages deployment?

@montymxb
Copy link
Contributor

montymxb commented Nov 28, 2017

@dplewis flovilmart is correct, it would be an overriding change. In the case of the php sdk we force gh-pages updates on deploying to the master only. It is a forced change which will overwrite existing changes in gh-pages. Depending on the setup this could be undesirable if you have people adding changes by hand from time to time, as their work could be undone.

In our case the entire set of docs is regenerated every time anyways. Automating it as part of a PR merging into master allows the docs to always be up to date as soon as changes are accepted. Just one less thing we have to do that we would be doing anyways, so it makes sense for us there. If it's a similar case here it might be equally useful to do. If anyone is (or will be) adding changes between updates it could be a problem though.

Another thing, I put something together over there that ensures that the doc that would be generated for a given PR must be valid. No more accidental missing docs for methods, vars, etc. If doc generation was automated here I would recommend something similar to help ensure a complete doc.

@flovilmart
Copy link
Contributor Author

Yep, that’s a good idea, but we loose library versioning :/

@montymxb
Copy link
Contributor

Hmm...if we want to preserve it we can checkout the existing gh-pages, archive the contents in a folder, and then proceed to make our brute-force update. If you did it right you could archive on the releases specifically, and end up with a nice set of folders containing the doc at that point in time. With the forefront docs being the latest set.

@flovilmart
Copy link
Contributor Author

That’s what I’m thinking about, i’ll Try to come up with a solution

@flovilmart flovilmart merged commit a209282 into master Nov 29, 2017
@flovilmart flovilmart deleted the jsdocs branch November 29, 2017 18:14
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.

5 participants