Skip to content

Conversation

jaketf
Copy link

@jaketf jaketf commented Nov 18, 2020

@danieldeleo this PR introduces the pre-commit I'd suggest for this repo as we discussed.
It is work in progress as there is a lot of work to be done to get the existing code to comply to the standards imposed by the pre-commit.

As the repo maintiner you'll have to make decisions about whether to ignore existing directories temporarily with exclude or fix all the non-compliant code in this PR. You may also have more opinions about the various flake8 / hadolint / yamllint / pytest configurations / ignore codes.

R: @ryanmcdowell
CC: @potiuk @mik-laj who inspired most of this on an earlier pso project

@jaketf jaketf requested a review from danieldeleo November 18, 2020 00:16
@jaketf
Copy link
Author

jaketf commented Nov 18, 2020

this gist gives an idea how much maintenance the current repo would need to be compliant w/ the suggested pre-commit hooks.
Optionally certain hooks could be commented out or certain directories excluded temporarily to make for smaller PRs to clean things up.

Jacob Ferriero added 4 commits November 19, 2020 11:04
# Conflicts:
#	tools/cloud_functions/README.md
#	tools/cloud_functions/gcs_event_based_ingest/gcs_ocn_bq_ingest/README.md
#	tools/cloud_functions/gcs_event_based_ingest/terraform_module/gcs_ocn_bq_ingest_function/README.md
#	tools/cloud_functions/gcs_event_based_ingest/terraform_module/gcs_ocn_bq_ingest_function/outputs.tf
#	tools/cloud_functions/gcs_event_based_ingest/terraform_module/gcs_ocn_bq_ingest_function/variables.tf
In the cloud funcitons module we had various python an terraform
static analysis steps that were redundant with the repo wide
pre-commit which will already be run by CI triggers. I've trimmed
it to include only the tests specific to the cloud function and
removed the duplicate static analysis configs in favor of the repo
wide configs.
@jaketf
Copy link
Author

jaketf commented Nov 19, 2020

pushed a few commits to get my own stuff inline and not run redundant checks

@jaketf jaketf mentioned this pull request Nov 19, 2020
@jaketf jaketf closed this Nov 19, 2020
@jaketf
Copy link
Author

jaketf commented Nov 19, 2020

closed in favor of #192

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.

1 participant