Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMake PropertyDeclarationBlock store a single vector of declarations #3869
Conversation
hoppipolla-critic-bot
commented
Nov 2, 2014
|
Critic review: https://critic.hoppipolla.co.uk/r/3055 This is an external review system which you may optionally use for the code review of your pull request. In order to help critic track your changes, please do not make in-place history rewrites (e.g. via |
| @@ -486,16 +457,14 @@ impl Stylist { | |||
| } | |||
|
|
|||
| struct PerOriginSelectorMap { | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
gilles-leblanc
Nov 4, 2014
Author
Contributor
Yeah makes sense. It should probably be deleted.
I'll add it to the list of things to modify on this pull request.
This comment has been minimized.
This comment has been minimized.
|
Reviewed on Critic. Please see https://critic.hoppipolla.co.uk/showcomment?chain=8473 in particular. |
|
@SimonSapin I've made another commit which corrects the I haven't found what to do for Since the If you can think of something I could implement I will wait your suggestion, if you would rather take it from now (as suggested in OperaCritic) that might also be a good idea since my time is severely limited for the next week and won't be able to put much time on this until next week. Implementing an iterator has got me thinking will iterating and filtering the fields incur a (even if small) performance cost? I remember reading that the selector_matching was somewhere performance needed to be good. |
|
@SimonSapin Is this something you're working on? Is it worth leaving this pull request open? |
|
It is in my queue but not at the top and I haven’t started yet. I don’t mind closing the PR (I just copied the branch) but is there a downside with leaving it open? |
|
I'm closing this as it is breaking bors. Apparently the original repo no longer exists. |
gilles-leblanc commentedNov 2, 2014
Changes PropertyDeclarationBlock to store a single vector instead of the
current two vectors; normal, important.
Fixes #3426