Skip to content
This repository was archived by the owner on Jul 6, 2019. It is now read-only.

Minimize use of DUMMY_SP #298

Merged
merged 1 commit into from
Jun 7, 2015

Conversation

mcoffin
Copy link
Contributor

@mcoffin mcoffin commented Jun 1, 2015

Makes more meaningful feedback from macro builders.

Fixes (for ioreg, not for platformtree) #99

@farcaller
Copy link
Member

We should have a lint warning for DUMMY_SP and unwrap() really.

@mcoffin
Copy link
Contributor Author

mcoffin commented Jun 1, 2015

While DUMMY_SP is pretty specific to compiler plugins (and I suspect it might be removed). I'll probably whip up an external lint plugin to check for unwrap().

@mcoffin mcoffin force-pushed the fix-99-minimize-dummy-sp branch 2 times, most recently from 0899d8f to 9a48067 Compare June 2, 2015 21:14
@mcoffin
Copy link
Contributor Author

mcoffin commented Jun 2, 2015

@farcaller FYI
https://github.com/mcoffin/syntaxext_lint

Before this gets merged, I'll publish the dependency crate to crates.io and we can avoid depending on my version from github.

@mcoffin mcoffin mentioned this pull request Jun 2, 2015
2 tasks
@farcaller
Copy link
Member

👏

@mcoffin mcoffin mentioned this pull request Jun 3, 2015
@mcoffin mcoffin force-pushed the fix-99-minimize-dummy-sp branch from 9a48067 to 5fc978b Compare June 3, 2015 09:57
@mcoffin
Copy link
Contributor Author

mcoffin commented Jun 3, 2015

Slightly changing the scope of this PR to only minimize DUMMY_SP for ioreg since platformtree might be coming up on a rework.

No longer completely fixes #99, but it goes a long way towards doing so.

@mcoffin mcoffin force-pushed the fix-99-minimize-dummy-sp branch from 5fc978b to b3d6e02 Compare June 3, 2015 10:14
@hacknbot
Copy link
Contributor

hacknbot commented Jun 3, 2015

Can one of the admins verify this patch?

@farcaller
Copy link
Member

ok to test

@mcoffin
Copy link
Contributor Author

mcoffin commented Jun 4, 2015

I can't see what went wrong with the jenkins build as it's behind a login screen. Probably just doesn't have the build script from the other branch?

@farcaller
Copy link
Member

Exactly, didn’t have enough time to squash and merge, will do that in an hour is so. Then it just needs to be rebased.

@farcaller
Copy link
Member

Okay, this can be rebased on master now.

@mcoffin
Copy link
Contributor Author

mcoffin commented Jun 7, 2015

Just got around to rebasing this as I'm pretty busy with finals through the end of the week. Opened #313 because of issues with the jenkins script.

@farcaller
Copy link
Member

retest this please

@farcaller
Copy link
Member

👍

farcaller added a commit that referenced this pull request Jun 7, 2015
@farcaller farcaller merged commit e5fd160 into hackndev:master Jun 7, 2015
@mcoffin mcoffin deleted the fix-99-minimize-dummy-sp branch June 8, 2015 19:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants