-
Notifications
You must be signed in to change notification settings - Fork 12
⚠️ CONFLICT! Lineage pull request for: skeleton #82
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
Merged
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
1e8f822
Add style enforcement rules
mcdonnnj afc6bd6
Add rule for image headers
mcdonnnj ce173f4
Switch to a YAML markdownlint configuration file
mcdonnnj f2a4230
Add comments to markdownlint configuration
mcdonnnj d4781ee
Add the validate_manifest hook from pre-commit
mcdonnnj 106af21
Install terraform and packer for the linting job
jsf9k c481043
Break out the curl cache creation into its own step
jsf9k 70414cf
Remove unnecessary line in tasks
jsf9k b629f7f
Modify the Packer installation to model that of Terraform
jsf9k 181d1b2
Install a specific version of terraform-docs
jsf9k bb6e566
Move go installation so that it takes place before the cache task
jsf9k 337d1ef
Capitalize Go for consistency
jsf9k 8ee2116
Prefer the newer "go install" syntax
jsf9k e2a729d
Install the shfmt tool for GHA
mcdonnnj 406b688
Replace the beautysh hook with pre-commit-shfmt
mcdonnnj 2b48e75
Apply changes from the shfmt pre-commit hook
mcdonnnj 1708b5c
Update pre-commit hooks
mcdonnnj d7dcfee
Merge pull request #83 from cisagov/maintenance/update_pre-commit_hooks
mcdonnnj c17800a
Merge branch 'develop' into improvement/add_pre-commit_hook
mcdonnnj 41a5286
Merge pull request #80 from cisagov/improvement/add_pre-commit_hook
mcdonnnj e607360
Merge branch 'develop' into improvement/update_mdl_configuration
mcdonnnj f6ad0e9
Merge branch 'develop' into improvement/install-tf-and-packer-for-lin…
jsf9k 382c39c
Merge pull request #79 from cisagov/improvement/update_mdl_configuration
mcdonnnj 3e3b918
Merge branch 'develop' into improvement/install-tf-and-packer-for-lin…
mcdonnnj 0e4fc41
Merge pull request #82 from cisagov/improvement/install-tf-and-packer…
mcdonnnj 3e83a80
Merge branch 'develop' into improvement/replace_beautysh_with_shfmt
mcdonnnj 81cdb4d
Merge pull request #84 from cisagov/improvement/replace_beautysh_with…
mcdonnnj e45aec3
Merge github.com:cisagov/skeleton-generic into lineage/skeleton
mcdonnnj 63cf76f
Changes from the pre-commit-shfmt hook
mcdonnnj 77c19cb
Add stubs required by mypy pre-commit hook
mcdonnnj 3452b9d
Install mypy type stubs as part of setup-env
jsf9k File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
--- | ||
|
||
# Default state for all rules | ||
default: true | ||
|
||
# MD003/heading-style/header-style - Heading style | ||
MD003: | ||
# Enforce the ATX-closed style of header | ||
style: "atx_closed" | ||
|
||
# MD004/ul-style - Unordered list style | ||
MD004: | ||
# Enforce dashes for unordered lists | ||
style: "dash" | ||
|
||
# MD013/line-length - Line length | ||
MD013: | ||
# Do not enforce for code blocks | ||
code_blocks: false | ||
# Do not enforce for tables | ||
tables: false | ||
|
||
# MD024/no-duplicate-heading/no-duplicate-header - Multiple headings with the | ||
# same content | ||
MD024: | ||
# Allow headers with the same content as long as they are not in the same | ||
# parent heading | ||
allow_different_nesting: true | ||
|
||
# MD029/ol-prefix - Ordered list item prefix | ||
MD029: | ||
# Enforce the `1.` style for ordered lists | ||
style: "one" | ||
|
||
# MD033/no-inline-html - Inline HTML | ||
MD033: | ||
# The h1 and img elements are allowed to permit header images | ||
allowed_elements: | ||
- h1 | ||
- img |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add these dependencies to
requirements-test.txt
orrequirements-dev.txt
so thatmypy
also works from the developer's local venv?If so, then is there any way to add a
requirements-mypy.txt
, say, that can be pulled in by bothrequirements-test.txt
(orrequirements-dev.txt
- I'm not certain where it should go) and ourpre-commit
configuration file? I'd prefer to add these dependencies in only one place.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be cleaner to leverage
mypy --install-types --non-interactive src/
insetup-env
to automatically install them to the local environment as part of setup.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since that also goes through type checking, we could eschew the
--non-interactive
part (which will also force a user to interact with thesetup-env
script during setup).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that makes sense. That's a change we'd want to make upstream, in skeleton-generic right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what you're saying here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry the
--non-interactive
step will also proceed withmypy
's type checking. Without that flag you have to do the--install-types
step and then runmypy
to do type checking. People wanted to do it all at once, but for our purposes it's ok to keep it broken up I believe.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently
mypy
is only a development requirement for projects descending from this skeleton. We can either add it here or add it to cisagov/skeleton-generic and addmypy
as a requirement as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, I see now that
mypy
isn't even installed in skeleton-generic.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See commit 3452b9d.