💡 Summary

Break out all of the Markdown specific guidelines in style-guide.md into an appropriate languages/markdown section.

Motivation and context

Currently the information in style-guide.md contains plain language guidelines intermixed with Markdown specific guidelines. I believe it would be better served to break out any Markdown specific guidelines into it's own language section. This will make it more obvious where to find the information as well as improving maintainability.

Implementation notes

The PR should be limited to creating a new languages/markdown section and only moving existing Markdown content in style-guide.md to this section. Any additions/modifications to the Markdown guidelines should be kept to their own PRs after this separation.

Acceptance criteria

• Create the PR
• Everyone approves the PR

💡 Summary

We should include manual installation steps for Terraform.

Motivation and context

Why does this work belong in this project?

Right now unless you run the laptop script instructed in the mac-env-setup users won't have TF installed on their systems. This could lead to some issues if they attempt to work any TF related project. The version should be pinned to the version specified in cisagov/setup-env-github-action.

Acceptance criteria

• Development guide includes steps of how to manually install TF.

💡 Summary

Run through the current project setup documentation and make sure it is up-to-date with current expectations and practices.

Motivation and context

The current instructions for project setup still reference the skeleton script in this repository. We have since migrated to using the gh CLI and the cisagov/gh-skeleton extension. There may be other out-dated our inaccurate items as well.

🐛 Summary

The current information in branch-protection.md about configuring required tests in branch protection settings no longer reflects the GitHub UI. The description should be updated to reflect that you must now search for tests to add them. Previously a list of tests was presented and you could check/uncheck for required tests.

We have a number of preferences for how the core dev team members use GitHub. This issue is to determine how we want to decide and codify these best-practices so they're well-documented and available.

Should we require dev team members to:

• Commit under their CISA or Trio email address?
  • Require this to be publicly available, or just to specify setting up the no-reply address for commit signing?
• Sign commits with a GPG key?
  • Possibly already codified in PR #35 with the inclusion of the FISMA-Ready Github guide, which we may want to fork and modify
  • Additional resource on setting up openPGP

Note: Specifically, this policy is for core dev team only, not external contributors, and should be written in a way not to discourage anyone from participating and using our GitHub resources

🚀 Feature Proposal

Unmaintained repositories should be archived. Ideally, people would do this but robots are much better at doing regularly-scheduled maintenance like that than people are. Let's:

1. Develop a policy for when unmaintained (i.e. un-updated) repositories get archived automatically
2. Implement a regularly-scheduled archive process for unmaintained repositories

Motivation

We want to make it clear when repositories are no longer maintained and reduce confusion and expectations. Code rot sets in as soon as code is written.

Repositories can be easily un-archived through the Settings menu and it will prompt someone to maybe do something about that repo.

Example

GSA 18F/TTS uses an automated archiving process to archive after 90 days of inactivity per their open source policy section on archiving.

💡 Summary

We should have a style guide for Terraform (HCL) code just as we do for Python.

Motivation and context

It's currently more just team lore as to what the preferences for how we write Terraform code. This include things like ordering statements, common permissions for common policies, and preferred naming. This will not only help the team write consistent Terraform code, but will also help in our collaboration with other developers who want to integrate with our Terraform-defined environments.

Implementation notes

This should be a new languages/terraform or languages/hcl (that may cause confusion because Packer has started moving to HCL and would likely want its own style guide). It can just contain some core elements at first and will give us a go-to place to add information as we develop further guidelines internally.

Acceptance criteria

• PR is created
• Everyone signs off on the PR

🐛 Summary

LGTM was a code-scanning tool that no-longer exists under that name. It is now built into GitHub Actions under the name, CodeQL. References to LGTM are in two files: /README.md and /languages/python/README.md.

Proposed Solution

1. Implement CodeQL scanning in all cisagov repositories (if not already done).
2. Update README files to replace references to LGTM with CodeQL.

Source

The next step for LGTM.com: GitHub code scanning!
About code scanning with CodeQL

🐛 Summary

When working on #57 I noticed that the section headings in project-setup/branch-protection.md are inconsistent. Some use a gerund phrase (e.g. "Selecting a skeleton") while others use the imperative mood (e.g. "Create an initial pull request"). I think the latter is preferred.

This is an easy enough change to make, but we will need to ensure that it does not result in any broken links. I believe this document is cross-referenced elsewhere.

💡 Summary

We should come to consensus on various Terraform-related coding/naming conventions and document them.

Motivation and context

In cisagov/cool-assessment-terraform#153 there was some discussion about how to best name Terraform resources that can contain one or multiple instances of that resource. We decided in that PR to pluralize the name of those resources and their related descriptions. If that is our decision, we should document it somewhere and abide by it.

Implementation notes

None

Acceptance criteria

• Terraform-related coding and/or naming conventions have been documented
• The above conventions can be applied to our code reviews going forward

💡 Summary

We do a decent job in this guide of describing the standards and practices we choose, but not such a great job of explaining why we chose them. We should make an effort to do so.

Motivation and context

In a dev team meeting today I mentioned that:

• Historically we do a terrible job of getting new folks up to speed. If someone is a strong self-starter then they usually do OK, but our current process strongly selects for self-starters.
• We have a difficult time getting contributors to read and follow the documentation in this repo.

It was mentioned that adding more explanation as to why we do things in the way we do might help with both issues.

🐛 Summary

When stepping through the "Setting up a Mac-based development environment" there is a sub section that seems to be outdated:
"Setup for the COOL". When I attempted to follow the steps each link that was listed just took me to the same COOL repo.

Also, when investigating the COOL repo, it seems that the repo will be decommissioned in a few months. This leads me to believe that maybe the instructions on the "Setup for the COOL" might need to be removed all together.

To reproduce

Steps to reproduce the behavior:

1. Go to this link https://github.com/cisagov/development-guide/blob/develop/dev_envs/mac-env-setup.md
2. Then scroll to the section "Setup for the COOL"
3. Attempt to setup the COOL

Expected behavior

User stepping through the guide should be able to follow the steps successfully to setup the COOL.

@hillaryj commented on Tue Oct 27 2020

This repository has no actual code testing, a .coveragerc file, or a test workflow, despite containing python scripts in the project_setup/scripts directory and requiring coverage in setup.py.

Acceptance Criteria

• Add test coverage for scripts
• Add .coveragerc file
• Integrate with coveralls

@mcdonnnj commented on Wed Oct 28 2020

I think we should discuss separating the guide from the tools. It would be nice if the tools were their own package to install instead in my opinion. This would make it easier to add new tools and have a functional CI/CD setup for those tools.

@jsf9k commented on Wed Oct 28 2020

I think we should discuss separating the guide from the tools. It would be nice if the tools were their own package to install instead in my opinion. This would make it easier to add new tools and have a functional CI/CD setup for those tools.

I think this is a grand idea. Do you want to create an issue so we don't lose track of this?

This repository has no actual code testing, a .coveragerc file, or a test workflow, despite containing python scripts in the project_setup/scripts directory and requiring coverage in setup.py.

Acceptance Criteria

• Add test coverage for scripts
• Add .coveragerc file
• Integrate with coveralls

That is a good point, although we devs tend to have a hard time knowing or remembering what normies know and don't know. It's definitely something to keep in mind when reviewing other devs' pull requests.

This triggered a thought: Should we have a document in our dev guide that talks about reviewing PRs and has a list of thangs like this to keep in mind? I think such a document could be useful, but could also turn into a list that is too long and random to be of any use. Does 18F or TTS have a document we can borrow/steal?

Originally posted by @jsf9k in cisagov/skeleton-docker#51 (comment)

          Do we need to escape periods here so they don't match any ol' character?

Originally posted by @jsf9k in #91 (comment)

🚀 Feature Proposal

Separate the Development Tools currently stored in project_setup/scripts into their own repository as a Python package.

Motivation

@hillaryj created an issue to add testing to the scripts in the above mentioned directory in #31 , but I believe it would be better to separate tooling from documentation.

Example

One would be able to install the development tools as a Python package a la pip install git+https://github.com/cisagov/development-tools.git and the scripts would be available in the environment.

Pitch

As it stands it is difficult to add testing and keep Python related skeleton updates current in this project because:

1. It descends from cisagov/skeleton-generic instead of cisagov/skeleton-python-library
2. It is a repository trying to store both documentation about our development process and the tools that are used

Moving the tooling into its own repository would allow it to be configured as a typical Python package, which would allow for our CI/CD pipeline to be implemented as it typically would for such a project. This will make it easier to add new tooling, eliminate duplicate code between tools, and ensure that testing is done automatically.

💡 Summary

Currently Docker no longer is a free to use for larger companies. See here for more details

An alternative like colima should be added to the development guide for developers looking for a free alternative.

Motivation and context

This belongs in here because currently the development guide provides steps for using Docker and no other alternative. To stay in the spirit of OSS we should provide an alternative that is free to use.

Acceptance criteria

• Mac-env-setup.md is updated to include Colima as an alternative.

🐛 Summary

The project_setup/scripts/skeleton script incorrectly replaces the name of the parent skeleton repository with the name of the new repository being created. This happens in the file .github/dependabot.yml.

For an example see the "before" here, paying special attention to the repository mentioned in the comment.

To reproduce

Steps to reproduce the behavior:

1. Use the project_setup/scripts/skeleton script to create from the skeleton-ansible-role skeleton repository a new repository named ansible-role-new.
2. Observe that cisagov/skeleton-ansible-role has been incorrectly replaced with cisagov/ansible-role-new in a comment in .github/dependabot.yml.

Expected behavior

The skeleton script should leave .github/dependabot.yml unaltered, apart from possibly uncommenting the Dependabot ignore directives that are managed by the parent repository.

💡 Summary

We should add to the development guide an explanation for why for Python package installation steps we typically lead with

python -m pip install --upgrade pip setuptools wheel

before installing other packages.

Motivation and context

We perform this step in most of our processes because of the need to upgrade pip and have wheel available. However, there is no explanation anywhere for why this is the case. We should document this here so that we have the rationale for this "on the record".

Implementation notes

Suitably explain why we upgrade pip and why we ensure setuptools and wheel are installed before proceeding with installing the Python packages actually required.

Acceptance criteria

• An approved pull request adding such documentation is approved and merged.

Original conversation

Is there a reason we can't/shouldn't consolidate this into a single line?

          python -m pip install --upgrade build pip setuptools wheel

Originally posted by @dav3r in cisagov/skeleton-python-library#95 (comment)

💡 Summary

Add steps to install packer as so users can take advantage of the setup-env script.

Motivation and context

We should include some info in our dev guide about installing packer. The version should be pinned to the version specified in cisagov/setup-env-github-action. Right now unless you run the laptop script instructed in the mac-env-setup users will run into an error. It can simply be fixed after running

brew install packer

Acceptance criteria

How do we know when this work is done?

• Development guide includes steps about manually installing packer

💡 Summary

Add some documentation for how to set up common tools for a development environment on Windows.

Motivation and context

Several people who work with our GitHub tooling are required to use Windows. Some of these users are also limited from installing some kinds of software without justifications/approval. Let's provide some guidance about how to do things anyway, such as:

• Windows Subsystem for Linux (WSL)
• Docker-based development images

Implementation details

This should be added to the dev_envs list.

Acceptance criteria

How do we know when this work is done?

• Documentation for how to setup and run standard skeleton tools on a Windows-based machine

🚀 Feature Proposal

We should add guidelines for repository naming to ensure that there is a consistent naming convention for repositories in the organization.

Motivation

We have provided a lot of guidance for development as an organization and I believe this should extend to naming conventions for repositories.

Pitch

I believe that consistent naming conventions make it easier to navigate a collection of projects. They set an expectation for what a repository name represents about a project. Having guidelines for naming encourages everyone to use the same conventions in our organization.

🐛 Bug Report

The terraform-to-secrets tool does not work with cisagov/molecule-iam-user-tf-module.

To Reproduce

Steps to reproduce the behavior:

1. terraform apply the code from the terraform directory in cisagov/ansible-role-kali/pull/13.
2. Run terraform-to-secrets from the same terraform directory.
3. Note that neither the user credentials nor the production role result in secrets being added to GitHub, although they are listed in the Terraform state.

Expected behavior

The secrets AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and TEST_ROLE_TO_ASSUME should be created.

Any helpful log output

The results of running the terraform-to-secrets tool from the ansible-role-kali/terraform directory:

(ansible-role-kali) jeremy_frasier@lemmy ~/cisagov/ansible-role-kali/terraform $ ~/cisagov/development-guide/project_setup/scripts/terraform-to-secrets -d -t e79b8cc9ac13ccbc4c4dba825fc1b92c15af8c4f
2020-05-17 12:38:56,211 INFO Using GitHub repository name: cisagov/ansible-role-kali
2020-05-17 12:38:56,211 INFO Reading state from Terraform command.
2020-05-17 12:38:59,385 INFO Searching Terraform state for IAM credentials.
2020-05-17 12:38:59,386 WARNING No users found.
2020-05-17 12:38:59,386 INFO Searching Terraform state for tagged resources.
2020-05-17 12:38:59,386 INFO Found secret: THIRD_PARTY_BUCKET_PRODUCTION
2020-05-17 12:38:59,386 INFO Found secret: THIRD_PARTY_BUCKET_STAGING
2020-05-17 12:38:59,386 INFO Creating GitHub API session using personal access token.
2020-05-17 12:38:59,387 INFO Requesting public key for repository cisagov/ansible-role-kali
2020-05-17 12:38:59,580 INFO Would create secret THIRD_PARTY_BUCKET_PRODUCTION
2020-05-17 12:38:59,581 INFO Would create secret THIRD_PARTY_BUCKET_STAGING
2020-05-17 12:38:59,582 INFO Success!

I've seen two major schools of thought on how to indicate that a PR that GitHub thinks is ready to merge actually needs to wait. This status is different than draft PRs because the code is ready to be reviewed/merged but may need other PRs to be completed before merging can be done, i.e. for coordinating a new feature across several repositories.

1. Preface the title with [HOLD] or another keyword
2. Include a label i.e. DO NOT MERGE or with the feature i.e. HOLD: COOL FEATURE or (preferably for standardizing) use the blocked label

Since we want to standardize repository labels in cisagov/.github#7, I'm recommending we adopt the title-preface method. We'll incorporate the results of discussion here into the team guide.