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 instructions for setting up git signing #4018

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

mgwalker
Copy link
Member

Changes proposed in this pull request:

  • Just what it says on the tin. This adds a new page to help folks get git signing setup.

@mgwalker mgwalker requested review from a team as code owners November 27, 2024 20:24
@@ -13,7 +13,7 @@
"test": "npm run test:prefixed-links && npm run test:internal-links && npm run test:html-validation && npm run test:spelling",
"test:html-validation": "html-validate _site/**/*.html",
"test:internal-links": "node check-links.js",
"test:prefixed-links": "! (grep -Erl \"\\(/|href=['\\\"]/\" pages && echo \"ERROR: Internal links must use {% page \"page name\" %} to work correctly with Cloud.gov Pages previews. Fix the above pages.\")",
"test:prefixed-links": "! (grep -Erl \"\\]\\(/|href=['\\\"]/\" pages && echo \"ERROR: Internal links must use {% page \"page name\" %} to work correctly with Cloud.gov Pages previews. Fix the above pages.\")",
Copy link
Member Author

Choose a reason for hiding this comment

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

This change tightens up the regex a little bit to look for ](/ to find relative links that are not using the {% page %} shortcode. The need comes from the case where we need to put file paths inside parentheses. For example:

Put the file in the magic directory (/Users/yourusername/.ssh)

The current regex flags that line as an invalid link, but it is, of course, not a link at all. Adding the preceding closing square bracket to the regex allows this line to pass while continuing to enforce the intent of the check.

The repo should reference the org where allstar is running from
Copy link
Member

@vrajmohan vrajmohan left a comment

Choose a reason for hiding this comment

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

Minor. TIL, KeePassXC has ssh integration.

[Allstar]({%link "https://github.com/GSA-TTS/.allstar" %}). If your team's
repository is managed by Allstar, it will _require_ commit signing.

We also strongly recommend using a password manager to manager your signing
Copy link
Member

Choose a reason for hiding this comment

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

"to manage"

Copy link
Member

@nateborr nateborr left a comment

Choose a reason for hiding this comment

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

I followed the KeyPass instructions and confirmed that git is signing my commits and GitHub marks them as verified! The manual steps look good to me although I didn't walk through them.

I added a couple of small and non-blocking notes on content. It looks good in terms of technical accuracy and overall flow! I'm setting this review at "Comment" pending another readability review.

[Allstar]({%link "https://github.com/GSA-TTS/.allstar" %}). If your team's
repository is managed by Allstar, it will _require_ commit signing.

We also strongly recommend using a password manager to manager your signing
Copy link
Member

Choose a reason for hiding this comment

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

necessary. Then enter this command in your terminal:

```sh
echo "your-email@gsa.gov [PASTE]" >> ~/.gitsigners
Copy link
Member

Choose a reason for hiding this comment

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

It might be pilot error but I didn't notice I also needed to update the your-email@gsa.gov part in this line the first time through, since the [PASTE] right next to it got my attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants