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

Documentation website #11

Merged
merged 6 commits into from
Jul 25, 2019
Merged

Documentation website #11

merged 6 commits into from
Jul 25, 2019

Conversation

dimamo5
Copy link
Collaborator

@dimamo5 dimamo5 commented Jul 15, 2019

This PR adds a documentation website to Velocidi Android SDK.

@dimamo5 dimamo5 requested a review from a team July 15, 2019 11:40
@ghost ghost requested review from beatriz and removed request for a team July 15, 2019 11:41
@dimamo5 dimamo5 changed the title Doc website Documentation website Jul 15, 2019
@codecov
Copy link

codecov bot commented Jul 15, 2019 •
edited
Loading

Codecov Report

Merging #11 into master will increase coverage by 3.8%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             master      #11     +/-   ##
===========================================
+ Coverage     69.48%   73.28%   +3.8%     
  Complexity        1        1             
===========================================
  Files             8        8             
  Lines           154      146      -8     
  Branches         21       20      -1     
===========================================
  Hits            107      107             
+ Misses           38       30      -8     
  Partials          9        9
Impacted Files Coverage Δ Complexity Δ
...c/main/kotlin/com/velocidi/GetAdvertisingIdTask.kt 14.28% <0%> (+7.61%) 0% <0%> (ø) ⬇️

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 6dba2de...5b3bbc8. Read the comment docs.

beatriz
beatriz requested changes Jul 18, 2019
View reviewed changes
Copy link
Contributor

@beatriz beatriz left a comment

Choose a reason for hiding this comment

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

LGTM overall just left a couple of small comments. When running, I noticed this:
Screenshot 2019-07-18 11 52 18
It would be nice if we could have a bit more spacing between the class name and its description.

docs/deploy.sh Outdated
# navigate into the build output directory
cd temp/.vuepress/dist

# if you are deploying to a custom domain
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you better specify these comments? The "if" seems that this is a configuration and we can set up to a custom domain or not, but I don't believe it is the case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 5b3bbc8

docs/deploy.sh Outdated

git commit -m 'deploy'

# if you are deploying to https://<USERNAME>.github.io/<REPO>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 5b3bbc8

docs/package.json Outdated
"generate-docs": "cd .. && ./gradlew :velocidi-sdk:dokka",
"import-docs": "cp -R ../velocidi-sdk/build/docs/velocidi-sdk/ temp && cp -R .vuepress temp",
"dev": "yarn run generate-docs && yarn run import-docs && vuepress dev temp",
"clean": "rm -R temp && rm -R ../velocidi-sdk/build/docs/",
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed offline:

Suggested change
"clean": "rm -R temp && rm -R ../velocidi-sdk/build/docs/",
"clean": "rm -Rf temp && rm -Rf ../velocidi-sdk/build/docs/",

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 5b3bbc8

@dimamo5
Copy link
Collaborator Author

dimamo5 commented Jul 18, 2019

LGTM overall just left a couple of small comments. When running, I noticed this:
Screenshot 2019-07-18 11 52 18
It would be nice if we could have a bit more spacing between the class name and its description.

I added a small margin in 5b3bbc8. Much better now :)

@dimamo5 dimamo5 requested a review from beatriz July 22, 2019 11:30
beatriz
beatriz approved these changes Jul 25, 2019
View reviewed changes
Copy link
Contributor

@beatriz beatriz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@beatriz beatriz merged commit 859d390 into master Jul 25, 2019
@beatriz beatriz deleted the doc-website branch July 25, 2019 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beatriz beatriz beatriz approved these changes

Assignees

@beatriz beatriz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dimamo5 @beatriz