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.

Sign up for GitHub

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

Jump to bottom

fix: enable package manifest file verification during PackageManifestFile::new #4412

Merged
merged 6 commits into from
Apr 11, 2023
Merged

fix: enable package manifest file verification during PackageManifestFile::new #4412

merged 6 commits into from
Apr 11, 2023

Conversation

kayagokalp
Copy link
Member

@kayagokalp kayagokalp commented Apr 7, 2023 •
edited
Loading

Description

Fixes missing validation step in the PackageManifestFile::new().
closes #4411.

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@kayagokalp kayagokalp added bug Something isn't working forc-pkg Everything related to the `forc-pkg` crate. labels Apr 7, 2023
@kayagokalp kayagokalp self-assigned this Apr 7, 2023
@kayagokalp kayagokalp marked this pull request as ready for review April 7, 2023 18:43
@kayagokalp kayagokalp enabled auto-merge (squash) April 7, 2023 18:44
@kayagokalp kayagokalp mentioned this pull request Apr 7, 2023
Merged
7 tasks
@kayagokalp kayagokalp requested a review from a team April 7, 2023 18:56
JoshuaBatty
JoshuaBatty previously approved these changes Apr 7, 2023
View reviewed changes
@JoshuaBatty JoshuaBatty requested a review from a team April 7, 2023 22:44
eightfilms
eightfilms reviewed Apr 10, 2023
View reviewed changes
Copy link
Contributor

@eightfilms eightfilms left a comment

Choose a reason for hiding this comment

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

LGTM other than a typo

forc-pkg/src/manifest.rs Outdated Show resolved Hide resolved
@eightfilms eightfilms requested a review from a team April 10, 2023 23:01
@kayagokalp @eightfilms
Update forc-pkg/src/manifest.rs
74c868b 
Co-authored-by: bing <bingcicle@proton.me>
eightfilms
eightfilms approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@eightfilms eightfilms left a comment

Choose a reason for hiding this comment

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

:shipit:

@eightfilms eightfilms requested a review from a team April 11, 2023 08:21
@kayagokalp kayagokalp merged commit 4c83b19 into master Apr 11, 2023
@kayagokalp kayagokalp deleted the kayagokalp/4411 branch April 11, 2023 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eightfilms eightfilms eightfilms approved these changes

@JoshuaBatty JoshuaBatty JoshuaBatty approved these changes

Assignees

@kayagokalp kayagokalp

Labels
bug Something isn't working forc-pkg Everything related to the `forc-pkg` crate.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PackageManifestFile verification never gets called
3 participants
@kayagokalp @JoshuaBatty @eightfilms