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

Improve and clean-up Scaladocs; resolves #184 #193

Merged
merged 28 commits into from Apr 11, 2018

Conversation

Projects
None yet
4 participants
@ruebot
Member

ruebot commented Apr 6, 2018

GitHub issue(s):

#184

What does this Pull Request do?

Improve and clean-up Scaladocs so that we will have much improved Scaladocs for the next release.

Every single scala file was touched.

How should this be tested?

  • TravisCI should take care of the build test
  • mvn site -DskipTests and visit target/site/scaladocs/index.html with a browser
  • We should do a smoke test with some auk standard output since we pulled in #189 and #186.

Interested parties

@lintool I'd appreciate your eyes on the doc comments in the code.
@ianmilligan1 can you do the smoke test, and any other review that you'd like to do?

Before this is merged let me know since we're squashing and merging this, the commit message is going to be pretty crazy with all the commits we've had go into this.

@greebie++ for his work on this.

ruebot and others added some commits Mar 20, 2018

@greebie

This comment has been minimized.

Show comment
Hide comment
@greebie

greebie Apr 6, 2018

Contributor

Just a small typo here. ext => text.

Just a small typo here. ext => text.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Apr 6, 2018

Codecov Report

Merging #193 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
- Coverage   67.34%   67.29%   -0.06%     
==========================================
  Files          32       32              
  Lines         637      636       -1     
  Branches      124      124              
==========================================
- Hits          429      428       -1     
  Misses        167      167              
  Partials       41       41
Impacted Files Coverage Δ
src/main/scala/io/archivesunleashed/package.scala 71.66% <ø> (ø) ⬆️
...io/archivesunleashed/matchbox/NER3Classifier.scala 0% <ø> (ø) ⬆️
...in/scala/io/archivesunleashed/util/JsonUtils.scala 100% <ø> (ø) ⬆️
...ala/io/archivesunleashed/matchbox/ComputeMD5.scala 100% <ø> (ø) ⬆️
...la/io/archivesunleashed/matchbox/ExtractDate.scala 60% <ø> (ø) ⬆️
.../io/archivesunleashed/matchbox/ExtractDomain.scala 100% <ø> (ø) ⬆️
...io/archivesunleashed/matchbox/DetectLanguage.scala 100% <ø> (ø) ⬆️
.../scala/io/archivesunleashed/app/WriteGraphML.scala 100% <ø> (ø) ⬆️
...scala/io/archivesunleashed/ArchiveRecordImpl.scala 81.57% <ø> (ø) ⬆️
...la/io/archivesunleashed/matchbox/ExtractUrls.scala 100% <ø> (ø) ⬆️
... and 18 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 3163ace...28fca42. Read the comment docs.

codecov bot commented Apr 6, 2018

Codecov Report

Merging #193 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
- Coverage   67.34%   67.29%   -0.06%     
==========================================
  Files          32       32              
  Lines         637      636       -1     
  Branches      124      124              
==========================================
- Hits          429      428       -1     
  Misses        167      167              
  Partials       41       41
Impacted Files Coverage Δ
src/main/scala/io/archivesunleashed/package.scala 71.66% <ø> (ø) ⬆️
...io/archivesunleashed/matchbox/NER3Classifier.scala 0% <ø> (ø) ⬆️
...in/scala/io/archivesunleashed/util/JsonUtils.scala 100% <ø> (ø) ⬆️
...ala/io/archivesunleashed/matchbox/ComputeMD5.scala 100% <ø> (ø) ⬆️
...la/io/archivesunleashed/matchbox/ExtractDate.scala 60% <ø> (ø) ⬆️
.../io/archivesunleashed/matchbox/ExtractDomain.scala 100% <ø> (ø) ⬆️
...io/archivesunleashed/matchbox/DetectLanguage.scala 100% <ø> (ø) ⬆️
.../scala/io/archivesunleashed/app/WriteGraphML.scala 100% <ø> (ø) ⬆️
...scala/io/archivesunleashed/ArchiveRecordImpl.scala 81.57% <ø> (ø) ⬆️
...la/io/archivesunleashed/matchbox/ExtractUrls.scala 100% <ø> (ø) ⬆️
... and 18 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 3163ace...28fca42. Read the comment docs.

@ruebot ruebot requested review from lintool and ianmilligan1 Apr 9, 2018

*
* @param verticesPath Filepath for vertices output
* @param edgesPath Filepath for edges output
* @return Unit().

This comment has been minimized.

@lintool

lintool Apr 9, 2018

Member

no trailing period.

@lintool

lintool Apr 9, 2018

Member

no trailing period.

This comment has been minimized.

@ruebot

ruebot Apr 9, 2018

Member

Trailing period goes on @return, but not on @param.

@ruebot

ruebot Apr 9, 2018

Member

Trailing period goes on @return, but not on @param.

/** Computes image size from a byte array using ImageIO.
*
* Used by `ExtractPopularImages` to calculate the size of

This comment has been minimized.

@lintool

lintool Apr 9, 2018

Member

Is backtick notation properly interpreted by scaladocs?

@lintool

lintool Apr 9, 2018

Member

Is backtick notation properly interpreted by scaladocs?

This comment has been minimized.

@ruebot

ruebot Apr 9, 2018

Member

Yes.

screenshot from 2018-04-09 08-02-31

@ruebot

ruebot Apr 9, 2018

Member

Yes.

screenshot from 2018-04-09 08-02-31

@lintool

Minor comments, okay to merge after cleanup...

@ianmilligan1

Looks great to me!

@lintool

This comment has been minimized.

Show comment
Hide comment
@lintool

lintool Apr 10, 2018

Member

lgtm

Member

lintool commented Apr 10, 2018

lgtm

@ruebot

This comment has been minimized.

Show comment
Hide comment
@ruebot

ruebot Apr 10, 2018

Member

@lintool did you want me to add Used by RecordLoader to extract data from WARC and ARC files. in place of ArchiveRecord. above, or it looks good as it is right now?

Member

ruebot commented Apr 10, 2018

@lintool did you want me to add Used by RecordLoader to extract data from WARC and ARC files. in place of ArchiveRecord. above, or it looks good as it is right now?

@lintool

This comment has been minimized.

Show comment
Hide comment
@lintool

lintool Apr 10, 2018

Member

either is fine.

Member

lintool commented Apr 10, 2018

either is fine.

@ruebot

This comment has been minimized.

Show comment
Hide comment
@ruebot

ruebot Apr 10, 2018

Member

@lintool cool. I'll add it, and then worth with @ianmilligan1 on getting it merged. Thanks for reviewing!

Member

ruebot commented Apr 10, 2018

@lintool cool. I'll add it, and then worth with @ianmilligan1 on getting it merged. Thanks for reviewing!

@ruebot ruebot dismissed stale reviews from lintool and ianmilligan1 via 28fca42 Apr 10, 2018

@ianmilligan1 ianmilligan1 merged commit 47f7a97 into master Apr 11, 2018

2 of 4 checks passed

codecov/patch 0% of diff hit (target 67.34%)
Details
codecov/project 67.29% (-0.06%) compared to 3163ace
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ianmilligan1 ianmilligan1 deleted the issue-184 branch Apr 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment