Skip to content

Various fixes #11250

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

Closed
wants to merge 7 commits into from
Closed

Various fixes #11250

wants to merge 7 commits into from

Conversation

emberian
Copy link
Member

@emberian emberian commented Jan 1, 2014

See the commits for details. From my triage queue.

@huonw
Copy link
Member

huonw commented Jan 1, 2014

Hm, would it be possible for the SNAP printing to be split out of tidy because the current output of tidy is extremely cluttered by the NOTE: snapshot printing, to the point that even experienced rustaceans will miss things like "trailing whitespace"?

e.g. add a target check-snaps so make check runs tidy, check-snaps, etc. (This would also allow check-snaps to run git grep SNAP -- '*.rs' to filter down to just relevant lines and then process this reduced subset of lines in Python, which will be much faster than iterating over the entirety of each file in Python, and so probably won't be much slower than only doing one pass (as is currently implemented).)

@emberian
Copy link
Member Author

emberian commented Jan 1, 2014

I'm not sure if we can use git grep, since we need to be able to make check
outside of the repo (tarballs).

On Tue, Dec 31, 2013 at 7:52 PM, Huon Wilson [email protected]:

Hm, would it be possible for the SNAP printing to be split out of tidybecause the current output of
tidy is extremely cluttered by the NOTE: snapshot printing, to the point
that even experienced rustaceans will miss things like "trailing
whitespace"?

e.g. add a target check-snaps so make check runs tidy, check-snaps, etc.
(This would also allow check-snaps to run git grep SNAP -- '*.rs' to
filter down to just relevant lines, which will be much faster than
iterating over each file in Python, and so probably won't be much slower.)


Reply to this email directly or view it on GitHubhttps://github.com//pull/11250#issuecomment-31416430
.

@emberian
Copy link
Member Author

emberian commented Jan 5, 2014

r? @huonw, @alexcrichton, @brson

I made tidy stop spitting out every NOTE line, unless it contains "snap". It really isn't useful. This might be a contentious change?

@@ -379,6 +379,18 @@ pub fn failing() -> bool {
local.get().unwinder.unwinding()
}

pub fn stack_bounds() -> Option<(uint, uint)> {
Copy link
Member

Choose a reason for hiding this comment

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

I think I commented on this commit as well, but let's leave this out for now.

I don't think it's relevant, a warning is more appropriate. Easier to just
maintain the order.

Closes #4681
This let's us specify exactly which snapshot a given note to update after
snapshot is for.

Closes #2483
pcwalton says this is right, and it looks right to me too.

Closes #4731
Obviously everything is unstable, but these particularly so, and they will
likely remain that way.

Closes #10239
@emberian emberian closed this Jan 6, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 31, 2023
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.

4 participants