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

remove deprecated 'branch' option #16301

Merged
merged 3 commits into from
Apr 24, 2024
Merged

remove deprecated 'branch' option #16301

merged 3 commits into from
Apr 24, 2024

Conversation

rmtolmach
Copy link
Contributor

@rmtolmach rmtolmach commented Apr 11, 2024

Summary

This updates an action in the k8s branch to remove a deprecated option. It was removed here: EndBug/add-and-commit@6fdb34e and we get warnings about it (here, for example. Note: it's not making anything fail)

Warning: Unexpected input(s) 'branch', valid inputs are ['add', 'author_name', 'author_email', 'commit', 'committer_name', 'committer_email', 'cwd', 'default_author', 'fetch', 'message', 'new_branch', 'pathspec_error_handling', 'pull', 'push', 'remove', 'tag', 'tag_push', 'github_token']

Trying new_branch. Not having a branch option was pushing this to all active branches, which we don't need.

https://dsva.slack.com/archives/C0460N83Y9G/p1713967398724749

What areas of the site does it impact?

k8s branch github action that pushes tag to manifest repo.

Testing

I'll watch CI after merge.

@rmtolmach rmtolmach requested a review from a team as a code owner April 11, 2024 17:14
Copy link

github-actions bot commented Apr 11, 2024

1 Error
🚫 This PR changes 1136 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, those exceeding
500 will not be reviewed, nor will they be allowed to merge. Please break this PR up into
smaller ones.

If you have reason to believe that this PR should be granted an exception, please see the
Submitting pull requests for approval - FAQ.

File Summary

Files

  • .github/workflows/build.yml (+182/-5)

  • .github/workflows/code_checks.yml (+0/-121)

  • .github/workflows/deploy-template.yml (+98/-0)

  • .gitignore (+1/-0)

  • Dockerfile (+51/-88)

  • Gemfile (+1/-1)

  • Makefile (+1/-1)

  • app/controllers/application_controller.rb (+0/-1)

  • app/uploaders/uploader_virus_scan.rb (+4/-3)

  • bin/deps (+1/-0)

  • bin/fake_clamdscan (+0/-5)

  • bin/test (+1/-0)

  • bin/test-setup (+2/-0)

  • clamav_tmp/1e7e39bfccbd672ca2fd00ead324b391 (+1/-0)

  • clamav_tmp/a0960199b278fd7e95f4df40f56046cd (+1/-0)

  • clamav_tmp/fe1ade8b718b283067640956690ffba6 (+1/-0)

  • config/ca-trust/README.md (+5/-1)

  • config/ca-trust/fwdproxy.crt (+24/-0)

  • config/clamd.conf (+4/-2)

  • config/freshclam.conf (+1/-1)

  • config/initializers/clamav.rb (+18/-0)

  • config/initializers/clamscan.rb (+0/-5)

  • config/initializers/datadog.rb (+0/-4)

  • config/settings.yml (+3/-0)

  • docker-compose-clamav.yml (+10/-0)

  • docker-compose-deps.yml (+8/-1)

  • docker-compose.yml (+46/-34)

  • docs/setup/hybrid.md (+40/-1)

  • docs/setup/native.md (+41/-22)

  • docs/setup/running_docker.md (+21/-0)

  • docs/setup/running_natively.md (+19/-7)

  • import-va-certs.sh (+5/-2)

  • lib/clamav/commands/patch_scan_command.rb (+43/-0)

  • lib/clamav/patch_client.rb (+76/-0)

  • lib/common/file_helpers.rb (+17/-0)

  • lib/common/models/redis_store.rb (+2/-2)

  • lib/common/virus_scan.rb (+12/-4)

  • lib/shrine/plugins/validate_virus_free.rb (+5/-3)

  • spec/lib/shrine/plugins/validate_virus_free_spec.rb (+9/-23)

  • spec/models/persistent_attachments/dependency_claim_spec.rb (+5/-2)

  • spec/models/persistent_attachments/lgy_claim_spec.rb (+5/-2)

  • spec/models/persistent_attachments/pension_burial_spec.rb (+5/-2)

  • spec/requests/claim_documents_spec.rb (+2/-2)

  • spec/simplecov_helper.rb (+0/-1)

  • spec/spec_helper.rb (+0/-1)

  • spec/support/uploader_helpers.rb (+1/-7)

  • spec/uploaders/uploader_virus_scan_spec.rb (+3/-7)

    Note: We exclude files matching the following when considering PR size:

    *.csv, *.json, *.tsv, *.txt, Gemfile.lock, app/swagger, modules/mobile/docs, spec/fixtures/, spec/support/vcr_cassettes/, modules/mobile/spec/support/vcr_cassettes/, db/seeds, modules/vaos/app/docs, modules/meb_api/app/docs, modules/appeals_api/app/swagger/, *.bru
    

Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@RachalCassity
Copy link
Member

@rmtolmach This commits the image tags to the manifest repo's main branch. The main branch should not be removed.

@rmtolmach
Copy link
Contributor Author

@RachalCassity I believe it commits to the default branch (main) by default. The branch option is deprecated.

@RachalCassity
Copy link
Member

The default branch would be k8s. Let me look into this more.

@rmtolmach
Copy link
Contributor Author

But it's pushing into the manifest repo.

This is definitely a low-priority item, it's just something I saw while on support. So if it feels too risky, we can just close it.

@RachalCassity
Copy link
Member

The new_branch allows customizing the branch name. In this workflow, the default branch would be k8s. We are sending this to another repo and the k8s branch does not exist. We can add main as the new_branch
new_branch: main

@rmtolmach
Copy link
Contributor Author

I want to reiterate that nothing is broken here. This PR would just remove the warning:
image

I don't know if we do need to add new_branch. From the README:

If you want the action to commit in a new branch, you can use the new_branch input.
Please note that if the branch exists, the action will still try push to it, but it's possible that the push will be rejected by the remote as non-straightforward.

main isn't a new branch, though.

By default the action runs the following command: git push origin ${new_branch input} --set-upstream.

So currently what it's running is git push origin --set-upstream and that seems to be working 🤷

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@rmtolmach rmtolmach merged commit 80cd349 into k8s Apr 24, 2024
15 of 18 checks passed
@rmtolmach rmtolmach deleted the rmtolmach-patch-4 branch April 24, 2024 20:18
@RachalCassity RachalCassity self-assigned this Apr 30, 2024
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.

4 participants