-
Notifications
You must be signed in to change notification settings - Fork 57
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
Innovation 2022Q4 - Add a github Wiki #1169
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice addition to the project! I've added some comments, they are non-blocking for me.
val task = target.tasks.register(GEN_TASK_NAME, GenerateWikiTask::class.java) { | ||
this.srcDir = File(File(File(target.buildDir, "reports"), "javadoc"), target.name) | ||
this.apiSurface = File(target.projectDir, "apiSurface") | ||
this.outputDir = File(target.buildDir, "wiki") | ||
this.projectName = target.name | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is no need to use target.afterEvaluate
and get task explicitly. With lazy API here it should be possible to declare task dependencies like the following:
val task = target.tasks.register(GEN_TASK_NAME, GenerateWikiTask::class.java) { | |
this.srcDir = File(File(File(target.buildDir, "reports"), "javadoc"), target.name) | |
this.apiSurface = File(target.projectDir, "apiSurface") | |
this.outputDir = File(target.buildDir, "wiki") | |
this.projectName = target.name | |
} | |
val task = target.tasks.register(GEN_TASK_NAME, GenerateWikiTask::class.java) { | |
this.srcDir = File(File(File(target.buildDir, "reports"), "javadoc"), target.name) | |
this.apiSurface = File(target.projectDir, "apiSurface") | |
this.outputDir = File(target.buildDir, "wiki") | |
this.projectName = target.name | |
dependsOn("dokkaGfm") | |
dependsOn(ApiSurfacePlugin.TASK_GEN_API_SURFACE) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea
githubwiki.py
Outdated
import requests | ||
from git import Repo | ||
|
||
OWNER = "xgouchet" # "DataDog" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be replaced before the merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, done
githubwiki.py
Outdated
def git_push_changes(repo: Repo): | ||
print("Pushing branch") | ||
origin = repo.remote(name="origin") | ||
repo.git.push("--set-upstream", "--force", origin, repo.head.ref) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need --force
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed we might not need it
if (typeDir.exists()) { | ||
println("Combining doc from $typeCanonicalName") | ||
combine(typeDir, outputFile, typeName) | ||
} else { | ||
System.err.println("Unable to find $typeDir") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is better to use logger
property here (and in other places) for logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, done
Codecov Report
@@ Coverage Diff @@
## master #1169 +/- ##
==========================================
- Coverage 83.19% 83.13% -0.06%
==========================================
Files 274 274
Lines 9458 9458
Branches 1540 1540
==========================================
- Hits 7868 7862 -6
Misses 1153 1153
- Partials 437 443 +6
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work !!
What does this PR do?
This PR adds an automated way to generate a global api reference documentation in the form of a Github Wiki
Additional Notes
The solution uses 3 steps :
master
branch)