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.

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

feat: update dev-tools documentation #1298

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Conversation

sdo-1A
Copy link
Contributor

@sdo-1A sdo-1A commented Jan 30, 2024

Proposed change

Update the documentation in the dev-tools package and the new packages during the dev-tools split. Some breaking changes are made as well.

@sdo-1A sdo-1A requested a review from a team as a code owner January 30, 2024 15:55
@sdo-1A sdo-1A force-pushed the feat/update-devtools-doc branch from c25f14b to 56e6a70 Compare January 30, 2024 16:02
@@ -1,272 +1,16 @@
# Otter utils library

Collection of tools to help to main a project with Otter Library
The Otter Framework provides a collection of tools to help to main a project with Otter Library
Copy link
Contributor

@cpaulve-1A cpaulve-1A Jan 30, 2024

Choose a reason for hiding this comment

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

Suggested change
The Otter Framework provides a collection of tools to help to main a project with Otter Library
The Otter Framework provides a collection of tools to help your Otter's project maintenance

@@ -3,32 +3,60 @@
<img src="https://raw.githubusercontent.com/AmadeusITGroup/otter/main/assets/logo/otter.png" alt="Super cute Otter!" width="40%"/>
</p>

This package is an [Otter Framework Module](https://github.com/AmadeusITGroup/otter/tree/main/docs/core/MODULE.md).
<br />
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

| `--basicAuth <base64>` | `-b` | `string` | | Base64 encoding of username:password (password already encrypted from artifactory UI) |
| `--dry-run <dryRun>` | | `boolean` | `false` | List all files that would be deleted without actually deleting them |
| `--duration-kept <durationKept>` | `-d` | `number` | `1` | All artifacts older than this value (in days) will be deleted |
| `--pr-versions <prVersions>` | `-pr` | `number` | `1` | Number of PR versions that will be kept |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is -pr working? it is not considered as -p -r?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested and -pr is working yes, but I can change the alias to a single letter if you prefer (like -n for example since we want the number of PR versions to keep)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think changing the alias would be actually safer

@sdo-1A sdo-1A force-pushed the feat/update-devtools-doc branch 2 times, most recently from 496d043 to 57a87f9 Compare February 2, 2024 10:22
@@ -20,7 +20,7 @@ program
.choices(['Replace', 'Add', 'Skip'])
.default('Add')
)
.option('-I, --threadIdentifier <threadIdentifier>', 'Thread identifier', undefined)
.requiredOption('-I, --threadIdentifier <threadIdentifier>', 'Thread identifier')
Copy link
Contributor

Choose a reason for hiding this comment

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

threadIdentifier should not be mandatory in case of mode="Add" (the default case)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it mandatory since it is passed as a parameter to the function findThreadsByIdentifier on line 65 and this parameter is of type string:

const threads = await prService.findThreadsByIdentifier(repositoryId, pullRequestId, opts.threadIdentifier);

Copy link
Contributor

Choose a reason for hiding this comment

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

 const threads = opts.threadIdentifier ? await prService.findThreadsByIdentifier(repositoryId, pullRequestId, opts.threadIdentifier) : []; 

@sdo-1A sdo-1A force-pushed the feat/update-devtools-doc branch from 57a87f9 to a34b790 Compare February 2, 2024 14:44
@sdo-1A sdo-1A added this pull request to the merge queue Feb 5, 2024
Merged via the queue into main with commit 7ad5533 Feb 5, 2024
25 checks passed
@sdo-1A sdo-1A deleted the feat/update-devtools-doc branch February 5, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants