Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Expose access to GetPropertyChangedSignal, take two #51

Merged
merged 8 commits into from
Mar 25, 2018

Conversation

AmaranthineCodices
Copy link
Contributor

After a hideously botched rebase attempt followed by a history rewrite happened in #37, I fixed things!

Same API as #37, but now with:

  • Update lemur to the newest commit
  • Fix a subtle but nasty bug that could cause property changes to almost never be connected
  • Test cases! 100% test coverage in SingleEventManager.

@coveralls
Copy link

coveralls commented Mar 17, 2018

Coverage Status

Coverage increased (+0.3%) to 87.719% when pulling 42e2415 on AmaranthineCodices:property-changes into 86a1ce3 on Roblox:master.

lib/Change.lua Outdated
setmetatable(Change, {
__index = function(self, propertyName)
local changeListener = {
type = "propertyChange",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should set the type to Change to line up with how Event works.

This also lines up really nicely with the other changes I want to make to typecheck arguments using the type value!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I missed that part of the refactoring! Fixed now.

return Reconciler
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like files' trailing newlines keep getting shuffled there and back, I feel like I should normalize those.

In the meantime, do you have an EditorConfig plugin enabled for your editor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do, but sometimes it doesn't work (see all the times my commits sometimes get spaces instead of tabs).

Copy link
Contributor

Choose a reason for hiding this comment

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

What editor do you use? The one for Sublime Text seems to horrifically break the automatic indentation detection. :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VS Code and this extension.

@AmaranthineCodices
Copy link
Contributor Author

This should be ready to go now!

@LPGhatguy
Copy link
Contributor

Looks good to me!

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.

3 participants