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

[CI] fix: run make go_mockgen step #63

Merged
merged 6 commits into from
Oct 18, 2023
Merged

[CI] fix: run make go_mockgen step #63

merged 6 commits into from
Oct 18, 2023

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Oct 13, 2023

Summary

Summary generated by Reviewpad on 17 Oct 23 23:06 UTC

This pull request includes the following changes:

Patch 1/5:

  • Fixes an issue by adding the make go_mockgen command to the build process.

Patch 2/5:

  • Removes the restriction of pulling requests only from the main branch.

Patch 3/5:

  • Modifies the Makefile to avoid failing when there is no code to remove.

Patch 4/5:

  • Deletes the localnet_config.yaml file, which was previously not ignored by git.

Patch 5/5:

  • Adds the make go_mockgen command to generate mocks for testing purposes.

Please review these changes.

Issue

Fixes CI failure in #59

CI did not have mockgen installed and needed to generate protobufs before running mockgen.

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make test_all_unit
  • Verify Localnet manually: See the instructions [here](TODO: add link to instructions)

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

@bryanchriswhite bryanchriswhite added testing Test (or test utils) additions, fixes, improvements or other infra Infra or tooling related improvements, additions or fixes labels Oct 13, 2023
@bryanchriswhite bryanchriswhite added this to the Shannon TestNet milestone Oct 13, 2023
@bryanchriswhite bryanchriswhite self-assigned this Oct 13, 2023
@bryanchriswhite bryanchriswhite force-pushed the fix/ci-mockgen branch 2 times, most recently from c34dc06 to 5a771df Compare October 13, 2023 15:09
@bryanchriswhite
Copy link
Contributor Author

bryanchriswhite commented Oct 13, 2023

NOTE: 👆CI is passing, next commit reverts temporary modification to add this branch to CI for testing.

image

@bryanchriswhite bryanchriswhite marked this pull request as ready for review October 13, 2023 15:21
Olshansk
Olshansk previously approved these changes Oct 16, 2023
@@ -27,8 +27,17 @@ jobs:
with:
go-version: "1.20"

- name: Generate protobufs
run: ignite generate proto-go
Copy link
Member

Choose a reason for hiding this comment

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

  1. I'm surprised that this works without out the --yes flag. Does the workflow automatically say yes?
  2. Consider using make proto_regen instead

@Olshansk
Copy link
Member

@bryanchriswhite After the minor change above, consider also basing thing against main so we can merge it in.

@okdas okdas changed the base branch from issues/8/application_stake to main October 17, 2023 21:50
@okdas okdas dismissed Olshansk’s stale review October 17, 2023 21:50

The base branch was changed.

@okdas okdas changed the base branch from main to issues/8/application_stake October 17, 2023 21:51
@okdas okdas changed the base branch from issues/8/application_stake to main October 17, 2023 21:57
@okdas okdas requested a review from Olshansk October 17, 2023 23:07
@bryanchriswhite bryanchriswhite merged commit 4be9ab9 into main Oct 18, 2023
3 checks passed
@bryanchriswhite bryanchriswhite deleted the fix/ci-mockgen branch October 18, 2023 14:30
bryanchriswhite added a commit that referenced this pull request Oct 19, 2023
* main:
  [Miner, Supplier] chore: scaffold submit-proof message (#44)
  [Application] Implement MsgUnstakeApplication & Add Extensive Tests (#72)
  [Gateway] Implement UnstakeGateway message and add Tests (#75)
  [Gateway] Implement StakeGateway Message and Add Tests (#68)
  [Application] Implement MsgStakeApplication & Add Extensive Tests (#59)
  [CI] fix: run `make go_mockgen` step (#63)
bryanchriswhite added a commit that referenced this pull request Oct 19, 2023
* feat/observable:
  chore: last minute improvements
  chore: review improvements
  chore: improve comment
  chore: misc. review feedback improvements
  chore: improve comments
  refactor: rename `Observable#Close()` to `#UnsubscribeAll()`
  chore: improve comments
  chore: improve comments
  chore: cleanup, simplification, review improvements
  [Miner, Supplier] chore: scaffold submit-proof message (#44)
  [Application] Implement MsgUnstakeApplication & Add Extensive Tests (#72)
  [Gateway] Implement UnstakeGateway message and add Tests (#75)
  [Gateway] Implement StakeGateway Message and Add Tests (#68)
  [Application] Implement MsgStakeApplication & Add Extensive Tests (#59)
  [CI] fix: run `make go_mockgen` step (#63)
okdas added a commit that referenced this pull request Nov 14, 2024
* fix: run make go_mockgen step

* run on all pull requests

* don't fail if there is no code to remove

* localnet config is not in gitignore yet :)

* add go_mockgen

---------

Co-authored-by: DK <[email protected]>
Co-authored-by: Dima Kniazev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Infra or tooling related improvements, additions or fixes testing Test (or test utils) additions, fixes, improvements or other
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants