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

Deduplicate warnings #68

Merged
merged 3 commits into from
Oct 18, 2022
Merged

Deduplicate warnings #68

merged 3 commits into from
Oct 18, 2022

Conversation

miraclx
Copy link
Contributor

@miraclx miraclx commented Oct 14, 2022

Fixes #67

Hides warnings at the ABI generation stage only when building contracts

Before After
CleanShot 2022-10-15 at 11 57 47@2x CleanShot 2022-10-15 at 11 55 35@2x

@miraclx miraclx requested a review from itegulov October 14, 2022 12:35
@miraclx miraclx linked an issue Oct 14, 2022 that may be closed by this pull request
@miraclx
Copy link
Contributor Author

miraclx commented Oct 14, 2022

@itegulov, when you get the time please can you test this. My one test in the screenshot worked, but ever since, I've been unable to get any warnings at all through cargo near, still investigating why. Thought it was a caching thing, but I've cleaned by target folder repeatedly and still nothing. And yes, normal cargo build reports errors.

maybe it's the meds I'm on

@itegulov
Copy link
Contributor

@miraclx yeah I am not getting any warnings either

@miraclx
Copy link
Contributor Author

miraclx commented Oct 14, 2022

Fixed in 5378499 (#65), merged in here.

Copy link
Contributor

@itegulov itegulov left a comment

Choose a reason for hiding this comment

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

Can confirm that warnings appear now!

rustflags.push_str(value);
}
_ => {
final_env.insert(key, value.to_string());
Copy link
Contributor

Choose a reason for hiding this comment

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

curious, why use a map instead of just a vec of (K, V) if you're not checking the duplication of keys on insert?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, actually, for RUSTFLAGS, we're checking to ensure we don't replace previous entries, but instead concatenate them. So it's here to allow us quickly index the map, instead of having to iterate the whole Vec<(K, V)>.

@miraclx miraclx force-pushed the miraclx/trigger-happy-stdout branch from 6b6b66c to 2b98c63 Compare October 18, 2022 07:47
Base automatically changed from miraclx/trigger-happy-stdout to main October 18, 2022 07:57
@miraclx miraclx force-pushed the miraclx/deduplicate-warnings-report branch from d099605 to 9907364 Compare October 18, 2022 08:01
@miraclx miraclx merged commit 9bb999e into main Oct 18, 2022
@miraclx miraclx deleted the miraclx/deduplicate-warnings-report branch October 18, 2022 08:03
@miraclx miraclx mentioned this pull request Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate warnings
3 participants