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

Add tools description #125

Merged
merged 8 commits into from
Apr 15, 2023
Merged

Add tools description #125

merged 8 commits into from
Apr 15, 2023

Conversation

thibaudrobin
Copy link
Contributor

Description

Hello there ! What about some tools description ?

Related issues

Roadmap : https://github.com/orgs/ThePorgs/projects/1/views/1?pane=issue&itemId=21406532

Point of attention

I hope there is no mistake !

@ShutdownRepo
Copy link
Member

Awesome! Will take a look asap

@ShutdownRepo ShutdownRepo added the documentation Improvements or additions to documentation label Mar 24, 2023
@ShutdownRepo ShutdownRepo changed the base branch from main to dev March 24, 2023 14:20
@ShutdownRepo
Copy link
Member

Cancelled checks because the target branch was wrong. Fixed branch, can you fix the conflicts?

Copy link
Member

@ShutdownRepo ShutdownRepo left a comment

Choose a reason for hiding this comment

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

There are many links that are wrong. Below are some examples. Can you review your PR and correct them?

@@ -1238,15 +1349,15 @@ function install_bloodhound_old_v3() {
unzip /tmp/BloodHound-linux-x64.zip -d /opt/tools/
mv /opt/tools/BloodHound-linux-x64 /opt/tools/BloodHound3
rm /tmp/BloodHound-linux-x64.zip
}
}#
Copy link
Member

Choose a reason for hiding this comment

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

why adding a # here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}# : this indicates this tool is duplicated and his version should not be documented because it would cause a duplication

Copy link
Member

Choose a reason for hiding this comment

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

alright, we should probably add a comment indicating that then

sources/install.sh Outdated Show resolved Hide resolved
sources/install.sh Outdated Show resolved Hide resolved
sources/install.sh Outdated Show resolved Hide resolved
sources/install.sh Outdated Show resolved Hide resolved
sources/install.sh Outdated Show resolved Hide resolved
sources/install.sh Outdated Show resolved Hide resolved
@Dramelac Dramelac added the waiting for additional changes Further changes are requested label Apr 3, 2023
@Dramelac
Copy link
Member

Dramelac commented Apr 3, 2023

Thank you for the help but there are still a lot of broken/wrong links ...

I'm putting this PR on hold. Let us know when it is fully ready for review and validation.

@thibaudrobin
Copy link
Contributor Author

I'm working on it

@thibaudrobin
Copy link
Contributor Author

Good ?

@thibaudrobin
Copy link
Contributor Author

Conflict solved

@ShutdownRepo
Copy link
Member

Before we go into review mode, did you check all links and descriptions to make sure they match?

@thibaudrobin
Copy link
Contributor Author

yep

@ShutdownRepo ShutdownRepo removed the waiting for additional changes Further changes are requested label Apr 13, 2023
@ShutdownRepo ShutdownRepo self-requested a review April 13, 2023 20:53
@ShutdownRepo ShutdownRepo merged commit 263442b into ThePorgs:dev Apr 15, 2023
@ShutdownRepo ShutdownRepo mentioned this pull request Apr 17, 2023
@ShutdownRepo ShutdownRepo linked an issue Apr 19, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List of tools
3 participants