search

stampit-org / stamp-specification

The Stamp Specification: Composables
434 stars 16 forks source link

fix issue #125, getter/setter will be success #126

Closed lornally closed 5 years ago

lornally commented 5 years ago

fix getter/setter error.

koresar commented 5 years ago

Wow! Looks like a simple fix. Let me check this change more thoroughly.

Before accepting the PR I would need to announce it and ask other people opinions.

Also, unit tests are lacking. I'll deal with it.

lornally commented 5 years ago

but, i can not fix it in stampit, because deep, static...... maybe, that all need rewrite, rewrite all code for properties clone.

lornally commented 5 years ago

i do a new pr, more exact for assign.

koresar commented 5 years ago

I have just gave it another thought. Please, take a look at this comment: https://github.com/stampit-org/stamp-specification/issues/7#issuecomment-122904939

Turns out the specification already support getters and setters. All we need is to keep them inside the propertyDescriptors metadata!

Thus, I believe that stamp-specification does not need change. Instead, we just need to improve the stampit module.

Sorry for the confusion @lornally . You have gave me the confidence. 👍 Thank you very much for that!!!

lornally commented 5 years ago

you are welcome. cool, we have propertyDescriptors, and actually, we have it in stampit, do we replace it in stampit? and ask a digression from this issue, how can you react so in time? you can receive some github message?