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: remove gno mod download #2673

Closed
wants to merge 1 commit into from

Conversation

zivkovicmilos
Copy link
Member

Description

This PR removes the useless, and frankly wrong gno mod download, removing the recent CI headaches we've had with chain querying from tests.

BREAKING CHANGE: absolutely useless command dropped

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@zivkovicmilos zivkovicmilos added breaking change Functionality that contains breaking changes 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Aug 9, 2024
@zivkovicmilos zivkovicmilos self-assigned this Aug 9, 2024
@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Aug 9, 2024
Copy link

codecov bot commented Aug 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.06%. Comparing base (bbcb2f6) to head (3aa5bee).
Report is 37 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2673      +/-   ##
==========================================
- Coverage   60.11%   60.06%   -0.06%     
==========================================
  Files         560      560              
  Lines       74918    74735     -183     
==========================================
- Hits        45039    44887     -152     
+ Misses      26504    26488      -16     
+ Partials     3375     3360      -15     
Flag Coverage Δ
contribs/gnodev 60.58% <ø> (-0.82%) ⬇️
contribs/gnofaucet 14.46% <ø> (-0.86%) ⬇️
gno.land 64.17% <ø> (ø)
gnovm 63.95% <ø> (-0.19%) ⬇️
misc/genstd 80.54% <ø> (ø)
misc/logos 20.23% <ø> (+0.35%) ⬆️
tm2 62.05% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@deelawn deelawn left a comment

Choose a reason for hiding this comment

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

I agree with you. I tried this out a few weeks ago and was confused by its behavior. It would be nice to eventually have a working gno mod download to better enable local development, but I think it's fine to remove it for now.

@zivkovicmilos zivkovicmilos added the don't merge Please don't merge this functionality temporarily label Aug 9, 2024
@x1unix
Copy link

x1unix commented Aug 9, 2024

Please don't remove that - Playground needs this command to download realm dependencies for a language server.

Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

we should not remove it, but improve it.

@zivkovicmilos
Copy link
Member Author

we should not remove it, but improve it.

@moul
Can you please clarify what you have in mind?
I've tried altering the pain points I had with this codebase (godawful FetchDeps tests), but eventually gave up because the core logic was beyond saving

@zivkovicmilos
Copy link
Member Author

Closing this PR as I haven't gotten clarity on what would be an alternative to this buggy functionality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Functionality that contains breaking changes don't merge Please don't merge this functionality temporarily 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants