-
Notifications
You must be signed in to change notification settings - Fork 44
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Much appreciated.
I see you added some additional commits - is this PR still in progress?
lib/impl/tooltip.dart
Outdated
@@ -116,19 +131,24 @@ class TooltipElement extends CoreElement { | |||
: super('div', classes: 'hover-tooltip') { | |||
id = 'hover-tooltip'; | |||
|
|||
print(position); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -26,9 +26,9 @@ | |||
"^1.0.0": "consumeToolbar" | |||
} | |||
}, | |||
"linter-plus-self": { | |||
"linter-indie": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update the required-packages list above as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on whether whatever is consuming the required-packages
key supports version numbers as that service is only available with linter
v2.
I landed the atom.dart plugin, so the travis build job now finishes cleanly. |
Oh damn... like I said I'm new to git, i thought the pull request would
use a snapshot copy... i am not done with reviving the tootips yet.
Should i try to uncommit those changes?
…On Thu, Jul 27, 2017 at 5:03 PM Devon Carew ***@***.***> wrote:
***@***.**** commented on this pull request.
Thanks for the PR! Much appreciated.
I see you added some additional commits - is this PR still in progress?
------------------------------
In lib/impl/tooltip.dart
<#1130 (comment)>:
> @@ -116,19 +131,24 @@ class TooltipElement extends CoreElement {
: super('div', classes: 'hover-tooltip') {
id = 'hover-tooltip';
+ print(position);
We should remove this.
------------------------------
In package.json
<#1130 (comment)>:
> @@ -26,9 +26,9 @@
"^1.0.0": "consumeToolbar"
}
},
- "linter-plus-self": {
+ "linter-indie": {
Do we need to update the required-packages list above as well?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1130 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AdDFw6iSkR7Qw_GKJq4J4qhnmy818m0Rks5sSKasgaJpZM4Ojrrg>
.
|
I would either revert those changes and update the PR, or keeping working and I can re-review when you're done. You'll have a bit of a git dance to do, so the 2nd option would likely be easier for you (given that you're new to git. I'm happy with whichever way you'd prefer - |
Thanks, I'll take option two :). It shouldn't be too long, probably some
time tomorrow.
…On Thu, Jul 27, 2017 at 6:51 PM Devon Carew ***@***.***> wrote:
Should i try to uncommit those changes?
I would either revert those changes and update the PR, or keeping working
and I can re-review when you're done. You'll have a bit of a git dance to
do, so the 2nd option would likely be easier for you (given that you're new
to git. Happy either way :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1130 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AdDFw5ODkepesE2Nn-JPCqby3uYlvTsFks5sSMAZgaJpZM4Ojrrg>
.
|
Moved to stream listeners. Changed tooltip to avoid updating if mouse is over same range.
A Better summary of this PR (which is now ready for review, sorry):
|
Awesome! Thanks much, landing. |
Fixed startup for atom 1.18
Fixed pub calls
Fixed linter/squiggles