cisagov / development-guide Goto Github PK
View Code? Open in Web Editor NEWA set of guidelines and best practices for an awesome engineering team
License: Creative Commons Zero v1.0 Universal
A set of guidelines and best practices for an awesome engineering team
License: Creative Commons Zero v1.0 Universal
Add some documentation for how to set up common tools for a development environment on Windows.
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:
This should be added to the dev_envs list.
How do we know when this work is done?
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.
[HOLD]
or another keywordDO NOT MERGE
or with the feature i.e. HOLD: COOL FEATURE
or (preferably for standardizing) use the blocked
labelSince 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.
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:
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.
GSA 18F/TTS uses an automated archiving process to archive after 90 days of inactivity per their open source policy section on archiving.
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.
The next step for LGTM.com: GitHub code scanning!
About code scanning with CodeQL
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)
We should have a style guide for Terraform (HCL) code just as we do for Python.
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.
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.
Run through the current project setup documentation and make sure it is up-to-date with current expectations and practices.
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.
Break out all of the Markdown specific guidelines in style-guide.md
into an appropriate languages/markdown
section.
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.
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.
@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.
.coveragerc
file@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?
We should include manual installation steps for Terraform.
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.
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.
Steps to reproduce the behavior:
project_setup/scripts/skeleton
script to create from the skeleton-ansible-role
skeleton repository a new repository named ansible-role-new
.cisagov/skeleton-ansible-role
has been incorrectly replaced with cisagov/ansible-role-new
in a comment in .github/dependabot.yml
.The skeleton
script should leave .github/dependabot.yml
unaltered, apart from possibly uncommenting the Dependabot ignore directives that are managed by the parent repository.
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.
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.
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.
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".
Suitably explain why we upgrade pip and why we ensure setuptools and wheel are installed before proceeding with installing the Python packages actually required.
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)
We should come to consensus on various Terraform-related coding/naming conventions and document them.
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.
None
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:
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
Add steps to install packer as so users can take advantage of the setup-env
script.
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
How do we know when this work is done?
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.
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.
Separate the Development Tools currently stored in project_setup/scripts into their own repository as a Python package.
@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.
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.
As it stands it is difficult to add testing and keep Python related skeleton updates current in this project because:
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.
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.
In a dev team meeting today I mentioned that:
It was mentioned that adding more explanation as to why we do things in the way we do might help with both issues.
Do we need to escape periods here so they don't match any ol' character?
Originally posted by @jsf9k in #91 (comment)
We should add guidelines for repository naming to ensure that there is a consistent naming convention for repositories in the organization.
We have provided a lot of guidance for development as an organization and I believe this should extend to naming conventions for repositories.
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.
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.
Steps to reproduce the behavior:
User stepping through the guide should be able to follow the steps successfully to setup the COOL.
The terraform-to-secrets
tool does not work with cisagov/molecule-iam-user-tf-module.
Steps to reproduce the behavior:
terraform apply
the code from the terraform
directory in cisagov/ansible-role-kali/pull/13.terraform-to-secrets
from the same terraform
directory.The secrets AWS_ACCESS_KEY_ID
, AWS_SECRET_ACCESS_KEY
, and TEST_ROLE_TO_ASSUME
should be created.
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!
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.
.coveragerc
fileA declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.