Fixed xfail for nbody shootout benchmark by correcting command line pars... by willingc · Pull Request #10410 · rust-lang/rust

willingc · 2013-11-11T05:29:24Z

...e.

Cleaned up unneeded imports and type changes to resolve compiler warnings.

Work in process

huonw · 2013-11-11T05:40:24Z

I have a feeling this isn't correct. It should probably either be left unchanged, or written as info!("thread: {id} got token: {token}", id=id, token=token);

Thanks for the feedback. I will check in the morning. Compiler warned on either mac osx or ubuntu -- can't remember which -- when left as is. Will double check on both systems. Either way, your suggestion above makes good sense.

huonw, I get a compiler error if written as info!("thread: {} got token: {}", id, token); or as suggested info!("thread: {id} got token: {token}", id=id, token=token);
error too many arguments to fmt! found 3, expected 1
I believe that the info! is using fmt! instead of format!. Please advise on how to correct. Thanks!

Hm, that's a little peculiar... fmt! is no longer in the main codebase (well it is, but it emits an error and stops compilation). Maybe you need to pull from mozilla/master and rebase on top of it?

huonw, ubuntu system was compiling using 0.8. Removed 0.8 from system and reinstalled pre-0.9. Compiles cleanly now with original info! format.

Oh, also, could you squash the commits together? (Just so there's not a "ghost" change that is added in one commit and removed in the next.)

huon, I tried to do the rebase -i and squash the commits together, but got lost in the land of git. Any suggestions on getting out of the muck of confusion? Thanks :)

huonw · 2013-11-12T11:38:09Z

The easiest way to fix this (that I (a non-git expert) know of) is probably to:

create a new branch from mozilla/master, say, shoot-done-2
git log to be able to compare later (will just be a check to see it all worked ok)
run git cherry-pick bc584ea (that's (a prefix of) the hash of the commit that you want) to copy that commit on to shoot-done-2.)
git log to check that everything is in order (that is, it should be your commit first and then the same sequence as from the first git log)

Then, if it is as you expect, you can either open a new PR from that new branch and close this one if you do (which is perfectly ok), or:

checkout the old branch (i.e. shoot-done)
run git reset --hard shoot-done-2 which will (destructively!*) update the current branch (shoot-done) to point to the clean version (i.e. shoot-done-2)
push the new clean version of shoot-done to your github fork, which should update this PR. (it will require a force push, i.e. git push -f origin shoot-done, where origin is the name of your own github remote repo.)

*It's not entirely destructive, one can use git reflog to recover the most recent commits to which a branch pointed.

For what it's worth, a "correct" procedure for just squashing the last two commits would be something like:

$ git rebase -i HEAD^^
# either 's'/'squash' or 'f'/'fixup'

the HEAD^^ means two commits above the tip of this branch (it could also be HEAD~2, or master if the branch was forked from master).

(Just guessing, it looks like you may have run git pull which merged your remote branch into your local branch, and caused confusion. In general, running either git fetch and manually rebasing/merging (e.g. git fetch mozillagit rebase mozilla/master to rebase a branch on top of the main master branch), or running git pull --rebase will have more controlled behaviour... at least, in my experience, I don't get tied up in knots so much when I avoid plain git pulls.)

(My apologies if this is aimed too high or too low, I don't have much context for your git experience. I'm happy to tune it either way, just ask.)

huonw · 2013-11-12T11:43:14Z

Also, there's no guarantee my instructions are 100% correct :( ... feel free to ask here or in IRC if things go awry again; I'm sure there'll be someone who can help. (Thanks for preserving with this.)

willingc · 2013-11-12T23:03:25Z

huon, thank you for the very detailed instructions. I think that things are now back how they should be - stuff squashed and cleaned up. I appreciate the time you took to coach me through this one :)

huonw · 2013-11-12T23:08:33Z

Not a problem at all.

Now this PR needs a rebase, so, something like git pull --rebase mozilla master (where mozilla is the name for the github.com/mozilla/rust remote).

…arse. Cleaned up unneeded imports and type changes to resolve compiler warnings.

willingc · 2013-11-12T23:22:35Z

Did the suggested rebase and then did git push -f origin shoot-done

Was that correct? [Thinking that I need to do some reading and drawing out of the the flow between the repos when doing rebase and pull, push, etc.]

I'll get it sooner or later. It's easier for me to grasp the visual flow vs. the commands ;)

...e. Cleaned up unneeded imports and type changes to resolve compiler warnings. Work in process

huonw · 2013-11-12T23:34:20Z

Looks correct to me!

I found http://pcottle.github.io/learnGitBranching/ a neat way of visualising git branches/trees/commits and learning (& practising) the various commands for manipulating them.

willingc · 2013-11-12T23:36:13Z

You rock!

I will check out your suggestion later this evening :)

Downgrade let_underscore_untyped to restriction From reading rust-lang#6842 I am not convinced of the cost/benefit of this lint even as a pedantic lint. It sounds like the primary motivation was to catch cases of `fn() -> Result` being changed to `async fn() -> Result`. If the original Result was ignored by a `let _`, then the compiler wouldn't guide you to add `.await`. **However, this situation is caught in a more specific way by [let_underscore_future](https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_future) which was introduced _after_ the original suggestion (rust-lang#9760).** In rust-lang#10410 it was mentioned twice that a <kbd>restriction</kbd> lint might be more appropriate for let_underscore_untyped. changelog: Moved [`let_underscore_untyped`] to restriction

…lp_message, r=Manishearth Improve the help message + add a help span This would closerust-lang#10410, because it applies the general consensus achieved in that issue (that replacing `let _ = ...` to `_ = ...` doesn't present any benefits). I also added a little help message span. changelog:[`let_underscore_untyped`]: Fix the help message confusion + add a help message span.

huonw reviewed Nov 11, 2013
View reviewed changes

Fixed xfail for nbody shootout benchmark by correcting command line p…
33ee433
…arse. Cleaned up unneeded imports and type changes to resolve compiler warnings.

bors added a commit that referenced this pull request Nov 12, 2013
auto merge of#10410: willingc/rust/shoot-done, r=huonw
2d6952e
...e. Cleaned up unneeded imports and type changes to resolve compiler warnings. Work in process

bors closed this Nov 13, 2013

bors merged commit 33ee433 into rust-lang:masterNov 13, 2013

Uh oh!

Uh oh!

Conversation

willingc commented Nov 11, 2013

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huonw commented Nov 12, 2013

Uh oh!

huonw commented Nov 12, 2013

Uh oh!

willingc commented Nov 12, 2013

Uh oh!

huonw commented Nov 12, 2013

Uh oh!

willingc commented Nov 12, 2013

Uh oh!

huonw commented Nov 12, 2013

Uh oh!

willingc commented Nov 12, 2013

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants