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

Fix clobbering of process vs component vs forc #668

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alfiedotwtf
Copy link
Contributor

These issues were found while investigating #654.

Currently, component_name was being repurposed for multiple functions:

  • Checking whether or not the executable was published by forc
  • Checking whether or not the executable was in the store
  • Used to generate the tarball target
  • Used to generate the tarball URL
  • Used to generate the final target executable to run from the store

Each of the above was breaking in subtle ways depending on which executable was being run.

After separating out these functions based on the properly resolved component from the executable, and along with the previous commits, these problems above were resolved.

@alfiedotwtf alfiedotwtf added bug Something isn't working code quality fuelup labels Oct 4, 2024
@alfiedotwtf alfiedotwtf requested a review from a team October 4, 2024 08:17
@alfiedotwtf alfiedotwtf self-assigned this Oct 4, 2024
@fuel-service-user
Copy link
Contributor

fuel-service-user commented Oct 4, 2024

LCOV of commit 7329e4e during CI #2088

Summary coverage rate:
  lines......: 85.6% (2457 of 2872 lines)
  functions..: 46.1% (382 of 829 functions)
  branches...: 63.1% (289 of 458 branches)

Files changed coverage rate: n/a

component/src/lib.rs Outdated Show resolved Hide resolved
component/src/lib.rs Outdated Show resolved Hide resolved
@JoshuaBatty JoshuaBatty requested a review from a team October 10, 2024 23:41
@alfiedotwtf alfiedotwtf marked this pull request as draft October 22, 2024 00:07
@alfiedotwtf alfiedotwtf changed the title Fix clobbering of process vs component vs forc (#654) Fix clobbering of process vs component vs forc Oct 27, 2024
- Checking whether or not the executable was published by forc
- Checking whether or not the executable was in the store
- Used to generate the tarball target
- Used to generate the tarball URL
- Used to generate the final target executable to run from the store

Each of the above was breaking in subtle ways depending on which
executable was being run.

After separating out these functions based on the properly resolved
component from the executable, and along with the previous commits,
these problems above were resolved.
@alfiedotwtf alfiedotwtf marked this pull request as ready for review October 29, 2024 01:05
@JoshuaBatty
Copy link
Member

Can we add some tests to show how this change works?

@alfiedotwtf
Copy link
Contributor Author

Can we add some tests to show how this change works?

Yep, no problem.

@alfiedotwtf alfiedotwtf marked this pull request as draft November 13, 2024 09:54
@alfiedotwtf
Copy link
Contributor Author

Keeping this PR in DRAFT but backgrounding for now...

90% of the tests have been done, I just have to do the other 90%. Backgrounding for now, but will leave this as a downtime task to complete.

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

Successfully merging this pull request may close these issues.

3 participants