#16848 closed enhancement (fixed)
[patch][cla] make createSubclass() mixins parameter optional
Reported by: | ben hockey | Owned by: | cjolif |
---|---|---|---|
Priority: | high | Milestone: | 1.10 |
Component: | Core | Version: | 1.8.3 |
Keywords: | Cc: | Bryan Forbes | |
Blocked By: | Blocking: |
Description (last modified by )
since the base class is implied in createSubclass
it should work without providing mixins.
SomeClass.createSubclass({ ... }); // should work SomeClass.createSubclass([], { ... }); // the current workaround
Attachments (1)
Change History (17)
comment:1 Changed 8 years ago by
Owner: | set to cjolif |
---|---|
Status: | new → assigned |
comment:2 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:3 Changed 8 years ago by
Cc: | Bryan Forbes added |
---|
comment:4 Changed 8 years ago by
Type: | defect → enhancement |
---|
comment:5 Changed 8 years ago by
That's true, however createSubclass(null, {})
doesn't even work, so it's not matching declare anyway.
Changed 7 years ago by
Attachment: | 0001-Made-declare.createSubclass-mixins-parameters-option.patch added |
---|
comment:6 Changed 7 years ago by
Here by I want to contribute a patch for this issue. Basically the mixins parameter is made optional same way as in the declare function. I think an extra test case is not needed for this issue. Let me know if you need anything.
EDIT: Ignore the attachment, was the wrong file. See here for patch: https://github.com/DevShaft/dojo/commit/9aab83215fb7190ff7f98ba6c2c38275c24c80f9.patch
comment:7 Changed 7 years ago by
Hi DevShaft?,
Thanks for your contribution. A few things however:
1/ you need to sign a Dojo CLA to contribute see http://dojofoundation.org/about/cla. Have you signed one? 2/ having a test would be good. Especially that this should be easy by just extending the current createSubclass tests with this? 3/ once 1/ and 2/ are done I suggest submitting your patch as a github pull request as this is the new way to proceed (see https://github.com/dojo/dojo/blob/master/CONTRIBUTING.md)
comment:8 Changed 7 years ago by
Hi cjolif, thanks for your reply. Yes, I have 1/ and will do 2/ and 3/.
comment:9 Changed 7 years ago by
Milestone: | tbd → 1.10 |
---|---|
Priority: | undecided → high |
Summary: | createSubclass assumes mixins are always provided → [patch][cla] createSubclass assumes mixins are always provided |
comment:10 Changed 7 years ago by
Looks like https://github.com/dojo/dojo/pull/46 has a test case (and the fix).
comment:11 Changed 7 years ago by
thanks bill, so if nobody has objections I'm going to merge this as I own the ticket.
comment:14 Changed 7 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:16 Changed 7 years ago by
Summary: | [patch][cla] createSubclass assumes mixins are always provided → [patch][cla] make createSubclass() mixins parameter optional |
---|
Would be nice, although you could also argue that declare({ ... }) should work, rather than declare([], { ... }).