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

Add forc-submit to components.toml (sway #3875) #689

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

alfiedotwtf
Copy link
Contributor

Sway's FuelLabs/sway#3885 added a new executable to submit transactions to the network, but it was missed in the fuelup repo.

@alfiedotwtf alfiedotwtf added the bug Something isn't working label Nov 15, 2024
@alfiedotwtf alfiedotwtf requested a review from a team November 15, 2024 08:25
@alfiedotwtf alfiedotwtf self-assigned this Nov 15, 2024
@alfiedotwtf alfiedotwtf marked this pull request as draft November 15, 2024 14:24
@alfiedotwtf
Copy link
Contributor Author

Weird...

Haven't dived in to find the reason behind this yet, but the tests are failing because Sway release binaries [1] contain forc-submit whereas the sway-nightly-binaries [2] don't 🤔

[1] https://github.com/FuelLabs/sway/releases/
[2] https://github.com/FuelLabs/sway-nightly-binaries/releases

@alfiedotwtf
Copy link
Contributor Author

With sway-nightly-binaries missing forc-submit is now fixed, I've run the build job to produce assets.

I'll update the yesterday() function to point to tomorrow so that this failing test will pick it up.

Then once tomorrow rolls over, all date hacks to yesterday() can then be removed.

@alfiedotwtf alfiedotwtf marked this pull request as ready for review November 16, 2024 11:00
@fuel-service-user
Copy link
Contributor

fuel-service-user commented Nov 16, 2024

LCOV of commit 1efdbb0 during CI #2097

Summary coverage rate:
  lines......: 85.7% (2457 of 2868 lines)
  functions..: 46.1% (382 of 829 functions)
  branches...: 63.0% (290 of 460 branches)

Files changed coverage rate: n/a

@zees-dev
Copy link
Contributor

Wondering if we need to add the forc-submit to the following places:

@alfiedotwtf
Copy link
Contributor Author

@zees-dev Good catch. The second two, yes. But the first one, although I'm not actually sure where the line is to classify something to go into forc or a plugin, thinking it's the end of the pipe to forc-tx which is also not in forc, I'd leave it under forc-client for now.

(it actually doesn't make any difference AFAICS in the code).

tests/show.rs Outdated
@@ -36,6 +36,7 @@ fn fuelup_show_latest() -> Result<()> {
- forc-client
- forc-deploy : 0.1.0
- forc-run : 0.1.0
- forc-submit : not found
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these show 0.1.0 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doh! Oh wow, ok I see where I went wrong... I assumed that file didn't exist because it was pulling down an old version of from githubusercontent.com where forc-submit doesn't exist, but they're actually coming from mocked files defined by ALL_BINS. I'll fix that, thanks!

@alfiedotwtf
Copy link
Contributor Author

Thanks @sdankel, much better!

@alfiedotwtf alfiedotwtf merged commit 665abb1 into master Nov 19, 2024
17 checks passed
@alfiedotwtf alfiedotwtf deleted the alfie/forc-submit branch November 19, 2024 03:09
@bachhoant
Copy link

665abb1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants