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

chore(scripts): simplify mock.gen.sh to use go generate commands #1413

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

qdm12
Copy link
Collaborator

@qdm12 qdm12 commented Dec 27, 2024

Why this should be merged

💁 Sibling PR for coreth so both repositories would look similar in this aspect

  • Remove license headers from generated mock files
  • Localized mockgen commands in the package where they are needed
  • Less shell scripting needed
  • Remove unneeded source commands for constants.sh and versions.sh
  • Change CI check to be as similar as coreth
  • Add check to detect mocks without an associated generate command

How this works

  • New mocks_generate_test.go file per package where mocks need to be generated, containing only //go:generate mockgen commands. Each command is relative to the current package directory
  • Remove ineffective and unneeded scripting from scripts/mocks.gen.sh and remove now unneeded scripts/mocks.mockgen.txt
  • Move CI check as a step in the tests job before running actual tests, since mocks should really be used for unit tests only, I think it makes sense to have that as a prerequisite step for them
  • CI check removes all mockgen generated files before re-generating them to detect any mockgen files without a generate command associated.

How this was tested

./scripts/mocks.gen.sh

Need to be documented?

No

Need to update RELEASES.md?

Not really

@qdm12 qdm12 changed the title chore(scripts): simplify mock.gen.sh to use go generate commands chore(scripts): simplify mock.gen.sh to use go generate commands Dec 27, 2024
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch from 3edebbc to a37996e Compare December 27, 2024 11:29
@qdm12 qdm12 marked this pull request as ready for review December 27, 2024 11:48
@qdm12 qdm12 requested review from ceyonur, darioush and a team as code owners December 27, 2024 11:48
license_header Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have any sibling PR for avalanchego? This is a direct copy from avalanchego. I'd prefer to keep workflows in repos (coreth/subnet-evm/avalanchego) as similar as possible.

- Add missing license headers
- Localized mockgen commands in the package where they are needed
- Less shell scripting needed
- Remove unneeded `source` commands for constants.sh and versions.sh
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch from 6517376 to 32b875c Compare December 30, 2024 16:24
@qdm12 qdm12 force-pushed the qdm12/mock-gen-script branch from 0c66511 to b001446 Compare December 30, 2024 17:09
(Note using source mode was compulsory due to CGO)
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