-
Notifications
You must be signed in to change notification settings - Fork 26
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
NONE: Add Browser's KDocs #60
base: master
Are you sure you want to change the base?
Conversation
@@ -2,6 +2,15 @@ package com.atlassian.performance.tools.infrastructure.api.browser | |||
|
|||
import com.atlassian.performance.tools.ssh.api.SshConnection | |||
|
|||
/** | |||
* Installs browser and WebDriver. See [Chrome] or [com.atlassian.performance.tools.infrastructure.api.browser.chromium.Chromium69]. | |||
* Browser must provide public no-args constructor. |
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.
They don't have to provide public constructor.
We only require this no-arg stuff, where we're forced to use reflection, that is in a cross-JVM communication between the local JVM and the remote VU JVMs. So only the VU SPIs require the nullary constructor.
@@ -26,6 +26,9 @@ Dropping a requirement of a major version of a dependency is a new contract. | |||
## [Unreleased] | |||
[Unreleased]: https://github.com/atlassian/infrastructure/compare/release-4.14.5...master | |||
|
|||
### Added | |||
- `com.atlassian.performance.tools.infrastructure.api.browser.Browser` KDocs. |
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.
Technically it adds behavioral contracts:
Browser
has to install WebDriver (we don't need this contract IMHO)SshConnection
targets Ubuntu - this one is critical for implementors, so we need it
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.
@dagguh Are you suggesting this should be explicitly stated in the changelog?
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.
Yeah, that's the diff.
I wanna empathize with lib consumers. As a consumer of a lib, I'm gonna look for useful stuff, e.g. "since when can I rely on it being Ubuntu?". Not gonna look for "when did KDocs appear"?
Docs themselves are a standalone description and changelog is a series of human-level diffs.
No description provided.