Page MenuHomePhabricator

eolian-mono: Provide constructor parameters based on the constructors section of the Eo files.
ClosedPublic

Authored by lauromoura on Jan 27 2019, 7:47 AM.

Diff Detail

Repository
rEFL core/efl
Branch
constructor_params
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9298
Build 7912: arc lint + arc unit
felipealmeida created this revision.Jan 27 2019, 7:47 AM
felipealmeida requested review of this revision.Jan 27 2019, 7:47 AM
lauromoura requested changes to this revision.Jan 27 2019, 5:31 PM

It seems arc squashed a bunch of commits from my branch into a single commit. I'll split the patch and update the diff accordingly.

This revision now requires changes to proceed.Jan 27 2019, 5:31 PM
lauromoura commandeered this revision.Jan 27 2019, 5:48 PM
lauromoura edited reviewers, added: felipealmeida; removed: lauromoura.
lauromoura updated this revision to Diff 18935.Jan 27 2019, 5:49 PM

Update patch after splitting.

Updating D7789: eolian-mono: Provide constructor parameters based on the constructors

section of the Eo files.

I've updated examples.git with new syntax from this diff.

segfaultxavi requested changes to this revision.Jan 28 2019, 8:09 AM

THANKS! Just a few comments:

  • There's about 250 of these warnings: XML comment on 'Efl.Canvas.AnimationPlayer.AnimationPlayer(Efl.Object)' has a param tag for 'init_cb', but there is no parameter by that name. You removed the callback but left its documentation in place :)
  • The new constructor parameters are undocumented.
  • Why are the parameter names duplicated? As in StyleStyle, WinNameName, WinTypeType ... The whole point was to make it super easy for people using an IDE, so the param names should be nice.
  • @felipealmeida The examples are now broken until this patch is landed... you should have gone the phab route :)
This revision now requires changes to proceed.Jan 28 2019, 8:09 AM
lauromoura updated this revision to Diff 19082.Jan 30 2019, 7:11 PM

Rebase and update after Xavi comments. Also split between required and optional parameters.

Updating D7789: eolian-mono: Provide constructor parameters based on the constructors

section of the Eo files.

Awesome!

Some minor comments:

  • The link to the setter in the parameter docs looks weird:
///<param name="WinName">The window name.<see cref="Efl.Ui.Win.SetWinName"/></param>

Please use See <see cref="Efl.Ui.Win.SetWinName"/>. Mind the punctuation and the initial whitespace :)

  • So now ALL constructor params are mandatory because NONE of them is marked as @optional in the EO files, correct? Looks like we have to modify the EO files then!
  • While you are working on this, could the other (internal) constructors be hidden? If they cannot be hidden, can at least they document what they do and have their parameters explained?

Update with better references, more info into internal constructors and changed parameters to start with lower case.

segfaultxavi accepted this revision.Feb 1 2019, 12:55 AM

Awesome!

This revision is now accepted and ready to land.Feb 1 2019, 12:55 AM
devilhorns closed this revision.Feb 1 2019, 11:02 AM