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

Smaller refactor svh #27461

Closed
wants to merge 1 commit into from
Closed

Conversation

richo
Copy link
Contributor

@richo richo commented Aug 1, 2015

This is the first half of the implementation of what I set out to do in #26629.

This will make the location of attrs in crates not affect their hash values any more, although it doesn't expose a public way to do this outside of an entire Svh yet, which is how I intended to implement the dupe attribute lint.

For posterity, I did the yolo science with: https://gist.github.com/richo/f750855a762804e2b3e9

r? @alexcrichton

Ping #14132

This makes the hash of a crate no longer whitespace dependant on it's
attributes

Ping rust-lang#14132
@richo
Copy link
Contributor Author

richo commented Aug 1, 2015

(The assertion that this closes #14132 was based on yolo-science™, updated to reflect that)

@brson
Copy link
Contributor

brson commented Aug 2, 2015

@bors r+ Nice improvements.

@bors
Copy link
Contributor

bors commented Aug 2, 2015

📌 Commit de81de5 has been approved by brson

@richo
Copy link
Contributor Author

richo commented Aug 2, 2015

@brson you'll need to r-, I ran make check overnight and it breaks some svh tests (in the same way that my other changes did, it's upset about the crates hashes changing)

@Gankra
Copy link
Contributor

Gankra commented Aug 2, 2015

@bors r-

On Sun, Aug 2, 2015 at 11:54 AM, Richo Healey notifications@github.com
wrote:

@brson https://github.com/brson you'll need to r-, I ran make check
overnight and it breaks some svh tests (in the same way that my other
changes did, it's upset about the crates hashes changing)


Reply to this email directly or view it on GitHub
#27461 (comment).

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to send a new PR with tests fixed!

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.

5 participants