feature/524_add-version-to-help-text#525
Conversation
redcatbear
left a comment
redcatbear
left a comment
There was a problem hiding this comment.
I did a prereview of my own changes. I found a broken skill link in the process. Hence the skill changes that have nothing to do with this PR per-se.
| ## Integration | ||
|
|
||
| OFT is typically integrated into CI builds via plugins: | ||
|
|
||
| - **Maven**: `openfasttrace-maven-plugin` | ||
| - **Gradle**: `openfasttrace-gradle` |
There was a problem hiding this comment.
Please create a section "Tracing" with subsections
- CLI
- Maven
- Gradle
Each section should describe how to run the tracing using that tool.
I propose to also provide & document a wrapper script oftw.sh (e.g. based on https://github.com/exasol/exasol-driver-ts/blob/document-csv-import/.github/workflows/oftw.sh) for simplifying usage in CI & locally.
There was a problem hiding this comment.
Added one section per variant.
| } | ||
| catch (final IOException exception) | ||
| { | ||
| LOGGER.warning("Error loading version from resource file: " + exception.getMessage()); |
There was a problem hiding this comment.
Also log stack trace?
There was a problem hiding this comment.
I don't think that is necessary in this case. It's a static resource anyway. Not much that can go wrong.
| @@ -1,4 +1,4 @@ | |||
| OpenFastTrace | |||
| OpenFastTrace ${version} | |||
There was a problem hiding this comment.
| OpenFastTrace ${version} | |
| OpenFastTrace version ${version} |
Avoid confusion in case the version is UNKNOWN
| assertAll(() -> assertThat(version, is(not("unknown"))), | ||
| () -> assertThat(version, is(not("${version}")))); |
There was a problem hiding this comment.
Make the unknown value case insensitive, add an assertion using a regex \d+\.\d+\.\d+
| <resource> | ||
| <directory>src/main/resources</directory> | ||
| <filtering>false</filtering> | ||
| <excludes> | ||
| <exclude>version.properties</exclude> | ||
| </excludes> | ||
| </resource> |
There was a problem hiding this comment.
Why is the file included and excluded?
| <resource> | |
| <directory>src/main/resources</directory> | |
| <filtering>false</filtering> | |
| <excludes> | |
| <exclude>version.properties</exclude> | |
| </excludes> | |
| </resource> |
| #### CLI Version | ||
| `dsn~cli.version~1` | ||
|
|
||
| The CLI provides the current version of OpenFastTrace read from the resource file `version.properties`. |
There was a problem hiding this comment.
Reading from a resource file is an implementation detail. If you want to be so specific, please also describe how the version gets into this file ;)
|
2 participants
Close #524.