feat: support fetching and showing fuels_version in fuelup show #410
Conversation
|
This should be tested, but currently the tests are not aware of the store yet - created #411 to track this and prioritizing that, since that will require changes to the test env setup that doesn't concern this PR as a whole. |
|
Seems reasonable |
| @@ -89,6 +90,7 @@ latest-{target} (default) | |||
| "# | |||
| ); | |||
| assert!(stdout.contains(expected_stdout)); | |||
| assert!(!stdout.contains("fuels versions")); | |||
There was a problem hiding this comment.
The heading "fuels versions" is a little confusing to me because I think of fuels as the SDK, not forc. Perhaps a better heading would be "Fuel component versions" ?
There was a problem hiding this comment.
Or even just "Component versions" or "Package versions" since the use is running fuelup, so the versions much be pertaining to fuel.
There was a problem hiding this comment.
The heading "fuels versions" is a little confusing to me because I think of fuels as the SDK
Yeah actually, that is the point! I think there was a past version of forc that deviated from fuels which resulted in incompatibility between certain SDK methods and forc itself. So the feature introduced in this PR is meant to serve as a sort of 'guide' as to what fuels version the component was tested/built against.
I agree though it might need better naming or more explanation behind what 'fuels versions' mean 🤔
|
I merged the other PR that introduces testing to this PR - apologies but i'm going to have to re-request reviews soon |
eightfilms
force-pushed
the
bing/show-fuels-version
branch
from
c50db64
to
0837b2e
Compare
closes #289
This doesn't really address the issue directly because it shows the compatible
fuelsversion on a component level rather than the toolchain level, which I think is probably better.I was contemplating between dealing with this on the toolchain-level or on the component-level, and it came down to where we wanted the logic for getting the fuels version to live.
The problem
Ultimately, trying to get the fuels version means we have to unavoidably fetch it from the
Cargo.tomldirectly, whether we solve it on the toolchain-level or component-level.On the toolchain-level, we could publish the info in our channels, but that means that on a component-level we do not know what
fuelsversion is compatible.On the component-level, we could fetch the info directly on the first installation, but that means in some areas of fuelup we have to hardcode the logic to fetch the info directly, which may be ugly (see). However this method is a lot more flexible and we can leverage the store by storing a
fuels_versiontextfile that is cached in the component directory, rather than store it in the toolchain directory.Solution
Ultimately I decided to go with shipping this feature using the latter method.
My justification:
In both circumstances we have to maintain some ugly logic that hard codes the URI of the
Cargo.tomlfile to fetch and read thefuelsversion, so either way we would have to maintain such code. If we had chosen to ship this feature on the toolchain-level, we would have this logic in thebuild-channelscript anyway. I'd say the upside of shipping this on the backend side means that we don't need newfuelupreleases in the case that the URI changes, and that could be a strong case for NOT taking this approach.The latter method also allows us to show the tested
fuelsversion per component instead of per toolchain, which I think is better DevEx for users.At this point of my thought process I'm not sure if there's any cataclysmic downside to doing it as such instead of the original proposed method. I'm still open minded as to the best approach, so let me know if there's some flaw in my thinking here!