Skip to content

Conversation

@spookyvision
Copy link
Contributor

implement #10070 in runnables

@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

r? @Veykril

EDIT: sorry, never mind.

Copy link
Contributor

@jonas-schievink jonas-schievink left a comment

Choose a reason for hiding this comment

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

Looks good! I haven't figured out a way to test this yet, since "Peek Related Tests" does not work on first and second in the following code:

fn first() {}
fn second() {}

macro_rules! m {
    ($test:ident -> $first:ident, $second:ident) => {
        #[test]
        fn $test() {
            $first();
        }

        mod module {
            #[test]
            fn $test() {
                crate::$second();
            }
        }
    };
}

m!(abc -> first, second);

@spookyvision
Copy link
Contributor Author

Looks good! I haven't figured out a way to test this yet, since "Peek Related Tests" does not work on first and second in the following code:

// ...

Does it depend on some other aspect of this ticket or another missing feature?

@jonas-schievink
Copy link
Contributor

Does it depend on some other aspect of this ticket or another missing feature?

I don't think it's part of #10070, but I'm not really sure what exactly the problem is either

@jonas-schievink
Copy link
Contributor

bors d+

Looks like CI will do the remaining review, feel free to r+ once that passes

@bors
Copy link
Contributor

bors bot commented Sep 9, 2021

✌️ spookyvision can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

Squashing should fix the CI error.

@spookyvision
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Sep 9, 2021
10184: multi-token mapping for runnables.rs (#10070) r=spookyvision a=spookyvision

implement #10070 in [`runnables`](https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide/src/runnables.rs)

Co-authored-by: Anatol Ulrich <anatol.ulrich@ferrous-systems.com>
Co-authored-by: Anatol Ulrich <45840+spookyvision@users.noreply.github.com>
@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

But you didn't squash..

@spookyvision
Copy link
Contributor Author

But you didn't squash..

sorry - how should I fix this now?

@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

bors r-

@bors
Copy link
Contributor

bors bot commented Sep 9, 2021

Canceled.

@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

Untested:

$ git rebase -i @~4
# edit the file to put `f` or `fixup` on the last three commits (if vim starts, you'll need to press `i` to insert, then quit with `ESC` and  `:wq`
$ git log # check that there's only one commit
$ git show # check that the commit looks correct
$ git push --force-with-lease

@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

Umm, that didn't come out right.

@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

Ah, sorry, that should have been fixup, not pick.

@lnicola

This comment has been minimized.

    Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
@spookyvision
Copy link
Contributor Author

spookyvision commented Sep 9, 2021

Untested:

# ...

sorry, I wasn't clear in my phrasing, I wanted to ask how to undo the bors action 😆 - that seems resolved after your r-?

(rebase help is also always appreciated, but I think I nailed it now, after making it much worse temporarily)

@jonas-schievink
Copy link
Contributor

bors r+

@lnicola
Copy link
Member

lnicola commented Sep 9, 2021

bors r-

I already filed and r+ed #10188.

sorry, I wasn't clear in my phrasing, I wanted to ask how to undo the bors action laughing - that seems resolved after your r-?

Oh, sorry.

@bors
Copy link
Contributor

bors bot commented Sep 9, 2021

Canceled.

@bors bors bot closed this in 92ce768 Sep 9, 2021
@spookyvision spookyvision deleted the 10070-runnables-2 branch September 9, 2021 15:19
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.

3 participants