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

refactor(techStackGeneric): improve readability #252

Merged
merged 6 commits into from
Mar 28, 2024

Conversation

ee7
Copy link
Contributor

@ee7 ee7 commented Mar 21, 2024

Description

A handful of miscellaneous refactoring-only changes while I was looking into #249:

  • Clarify walkDir logic
  • Prefer proc over template
  • Remove unused variables
  • Fix indentation

Testing

The existing tests should be sufficient.

miki725
miki725 previously approved these changes Mar 21, 2024
Base automatically changed from ee7/refactor-techStackGeneric to main March 21, 2024 17:28
@ee7 ee7 dismissed miki725’s stale review March 21, 2024 17:28

The base branch was changed.

@ee7 ee7 force-pushed the ee7/refactor-techStackGeneric2 branch from 3f20271 to f9e785a Compare March 21, 2024 17:30
@ee7 ee7 marked this pull request as ready for review March 21, 2024 17:30
@ee7 ee7 requested a review from viega as a code owner March 21, 2024 17:30
@ee7
Copy link
Contributor Author

ee7 commented Mar 21, 2024

CI previously passed on this PR, but now fails despite no change to the PR. I believe the failure is unrelated:


main.go:62: error during command execution: unknown flag: --output-key-prefix

subprocess.CalledProcessError: Command '['cosign', 'generate-key-pair', '--output-key-prefix', 'chalk']' returned non-zero exit status 1.

[...]

Error: unknown flag: --tlog-upload

[...]

=========================== short test summary info >============================
FAILED test_command.py::test_setup_existing_keys[copy_files0-config1-False]
FAILED test_command.py::test_setup[config0-copy_files0] - subprocess.CalledPr...
FAILED test_command.py::test_setup[config1-copy_files0] - subprocess.CalledPr...
FAILED test_command.py::test_setup[config2-copy_files0] - subprocess.CalledPr...
FAILED test_command.py::test_setup_existing_keys[copy_files0-config0-True] - ...
======= 5 failed, 136 passed, 14 skipped, 1 warning in 273.03s (0:04:33) =======

@ee7
Copy link
Contributor Author

ee7 commented Mar 22, 2024

CI passed the second time on f3c2137, I believe due to upstream fixing their version pinning of latest (so CI passes in this PR even without pinning the cosign version in the tests).

@ee7 ee7 requested a review from miki725 March 22, 2024 17:51
@ee7
Copy link
Contributor Author

ee7 commented Mar 27, 2024

Happy to approve this again @miki725?

No intra-PR changes since your last approval, which got dismissed due to the branch change. I like stacked PRs, and prefer to merge them bottom-up, but I've never been happy with GitHub's handling of them.

@ee7 ee7 merged commit a3ae9f5 into main Mar 28, 2024
2 checks passed
@ee7 ee7 deleted the ee7/refactor-techStackGeneric2 branch March 28, 2024 09:55
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.

2 participants