Skip to content
This repository was archived by the owner on Apr 20, 2026. It is now read-only.

Option to ignore warnings from dependencies in foundry.toml#69

Merged
mattsse merged 19 commits intofoundry-rs:mainfrom
loocapro:ignore/dependencies/warnings
Feb 9, 2024
Merged

Option to ignore warnings from dependencies in foundry.toml#69
mattsse merged 19 commits intofoundry-rs:mainfrom
loocapro:ignore/dependencies/warnings

Conversation

@loocapro
Copy link
Copy Markdown
Contributor

@loocapro loocapro commented Feb 5, 2024

mattsse
mattsse suggested changes Feb 7, 2024
View reviewed changes
Comment thread src/compile/output/mod.rs Outdated
@loocapro loocapro requested a review from mattsse February 7, 2024 14:06
mattsse
mattsse suggested changes Feb 8, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

nice, this is pretty close.

only have some style nitpicks

Comment thread src/compile/output/mod.rs Outdated
Comment thread src/compile/output/mod.rs Outdated
Comment thread src/compile/output/mod.rs Outdated
Comment thread src/compile/output/mod.rs Outdated
Comment thread src/compile/output/mod.rs Outdated
loocapro and others added 3 commits February 8, 2024 15:50
@loocapro @mattsse
Update src/compile/output/mod.rs
1110c08 
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
@loocapro
review: contains to starts_with
c1766c9
@loocapro
review
da4b124
DaniPopes
DaniPopes reviewed Feb 8, 2024
View reviewed changes
Comment thread src/artifacts/mod.rs Outdated
@loocapro loocapro requested review from DaniPopes and mattsse February 8, 2024 15:35
mattsse
mattsse suggested changes Feb 8, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

almost there :)

Comment thread src/compile/output/mod.rs Outdated
Comment thread src/compile/output/mod.rs Outdated
@loocapro loocapro marked this pull request as ready for review February 8, 2024 16:17
@loocapro loocapro requested a review from Evalir as a code owner February 8, 2024 16:17
mattsse
mattsse suggested changes Feb 8, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

nice, let's add a simple test for this

todo:

add another test similar to this:

compilers/tests/project.rs

Line 1846 in 564eb85

fn can_apply_libraries_with_remappings() {

The lib code should trigger a warning

configure ignored paths and check output diagnostic

DaniPopes
DaniPopes suggested changes Feb 8, 2024
View reviewed changes
Comment thread src/artifacts/mod.rs
Comment thread src/compile/output/mod.rs Outdated
Comment thread src/compile/output/mod.rs Outdated
Comment thread src/artifacts/mod.rs Outdated
Comment thread src/compile/output/mod.rs Outdated
Comment thread tests/project.rs Outdated
@loocapro loocapro requested review from DaniPopes and mattsse February 8, 2024 20:08
DaniPopes
DaniPopes approved these changes Feb 8, 2024
View reviewed changes
Comment thread src/compile/output/mod.rs
mattsse
mattsse approved these changes Feb 9, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

@mattsse mattsse merged commit c42032f into foundry-rs:main Feb 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Evalir Evalir Awaiting requested review from Evalir

2 more reviewers

@mattsse mattsse mattsse approved these changes

@DaniPopes DaniPopes DaniPopes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@loocapro @mattsse @DaniPopes