cisagov / gophish-tools

Helpful tools for interacting with a GoPhish phishing instance
Creative Commons Zero v1.0 Universal
42 stars 6 forks source link

README JSON reference improvement #3

Closed bjb28 closed 4 years ago

bjb28 commented 4 years ago

Modify example/reference files for the assessment JSON and included information on README.md.

🗣 Description

💭 Motivation and Context

Improves the usability of field dictionary as the new format is easier to follow. Additionally, the changes make it easier to find the sample assessment JSON from the README.

🧪 Testing

Ensured all new links on the README.md functioned as expected.

📷 Screenshots (if appropriate)

🚥 Types of Changes

✅ Checklist

bjb28 commented 4 years ago

The backticks are unnecessary here and in a couple of other places below where you put strings in the backticks. If the input is a string, quotes are sufficient (even for URLs). In this case (list of strings), it's fine to simply write: ["target1.tld", "target2.tld"]

Do you have a preferred way to prevent links from being clicked? I have found a few ways to do it but want to make sure it fits.

dav3r commented 4 years ago

The backticks are unnecessary here and in a couple of other places below where you put strings in the backticks. If the input is a string, quotes are sufficient (even for URLs). In this case (list of strings), it's fine to simply write: ["target1.tld", "target2.tld"]

Do you have a preferred way to prevent links from being clicked? I have found a few ways to do it but want to make sure it fits.

Ah, I had forgotten about the clickable links. We don't have any standard for that- whatever is simplest. If that means backticks, I am suddenly completely on board with them. 😄

bjb28 commented 4 years ago

Changes have been made to place all strings in quotes. For URLs and email addresses that became clickable, I placed a single <span> tag within as this created the cleanest appearance in my opinion.

All changes were made in commit d1cd1d6.

dav3r commented 4 years ago

Awesome- this is looking great now, thanks for all of your work here! 🎸 ⭐

Ack- I spoke too soon. I just noticed that the <span> tags that you put in go against our Markdown linter rules (see failed lint step here):

README.md:82:53 MD033/no-inline-html Inline HTML [Element: span]
README.md:101:62 MD033/no-inline-html Inline HTML [Element: span]
README.md:124:141 MD033/no-inline-html Inline HTML [Element: span]

@bjb28 - Can you please change that back to either backticks or simply a quoted string (the fake hyperlinks don't really bother me that much).

bjb28 commented 4 years ago

Awesome- this is looking great now, thanks for all of your work here! 🎸 ⭐

Ack- I spoke too soon. I just noticed that the <span> tags that you put in go against our Markdown linter rules (see failed lint step here):

README.md:82:53 MD033/no-inline-html Inline HTML [Element: span]
README.md:101:62 MD033/no-inline-html Inline HTML [Element: span]
README.md:124:141 MD033/no-inline-html Inline HTML [Element: span]

@bjb28 - Can you please change that back to either backticks or simply a quoted string (the fake hyperlinks don't really bother me that much).

Removed <span> in commit 0689bda.