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 vendor dependencies and cleanup scripts. #11

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

jorgecuesta
Copy link
Collaborator

Summary

This pull request involves a significant change to our project's dependencies. We have used forked versions of the cosmjs and subql packages.

The primary reason behind opting for these forks was to address the compatibility issues with Cosmos SDK version 0.50.x . The official maintainers have since resolved these issues.

As such, I have removed the forked cosmjs and subql packages and replaced them with the official ones. This change aims to ensure our project remains current, reliable, and secure. Additionally, using the official packages enables us to take advantage of future improvements and bug fixes more efficiently.

I have thoroughly tested these changes and confirmed that they don't introduce any adverse effects on our project's functionality.

Type of change

Select one or more:

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

Testing

Documentation changes (only if making doc changes)

  • make docusaurus_start; only needed if you make doc changes

Local Testing (only if making code changes)

  • Unit Tests: make go_develop_and_test
  • LocalNet E2E Tests: make test_e2e
  • See quickstart guide for instructions

PR Testing (only if making code changes)

  • DevNet E2E Tests: Add the devnet-test-e2e label to the PR.
    • THIS IS VERY EXPENSIVE, so only do it after all the reviews are complete.
    • Optionally run make trigger_ci if you want to re-trigger tests without any code changes
    • If tests fail, try re-running failed tests only using the GitHub UI as shown here

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have commented my code
  • I have performed a self-review of my own code; both comments & source code
  • I create and reference any new tickets, if applicable
  • I have left TODOs throughout the codebase, if applicable

SubQL latest release support all the fixes and enhancements that was made by @bryanchriswhite on vendor packages. So there is no need to keep them on the repo and overcomplicate the build process.
@jorgecuesta jorgecuesta added the enhancement New feature or request label Aug 5, 2024
@jorgecuesta jorgecuesta requested a review from Olshansk August 5, 2024 21:11
@jorgecuesta jorgecuesta self-assigned this Aug 5, 2024
README.md Show resolved Hide resolved
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

@jorgecuesta LGTM but please make sure the CI passes before merging it in!

Ensure both architectures are properly built and tested for CI compatibility. Introduced a precommit hook to streamline the process and made necessary amendments to the Dockerfile for ARM64 support.
@jorgecuesta jorgecuesta requested a review from Olshansk August 6, 2024 16:21
@jorgecuesta
Copy link
Collaborator Author

@jorgecuesta LGTM but please make sure the CI passes before merging it in!

Fixed

@Olshansk Olshansk merged commit 35a54b2 into main Aug 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants