Exclude function properties from generated components #4673
Conversation
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained
flow-components-parent/flow-code-generator-api/src/main/java/com/vaadin/generator/ComponentGeneratorUtils.java, line 388 at r1 (raw file):
case FUNCTION: System.out.println("asdf");
asdf for you too
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained
flow-components-parent/flow-code-generator-api/src/main/java/com/vaadin/generator/ComponentGeneratorUtils.java, line 388 at r1 (raw file):
Previously, gilberto-torrezan (Gilberto Torrezan) wrote…
asdf for you too
ty <3
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, 1 of 1 files at r3.
Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained
a discussion (no related file):
Also use the code generator and push the new sources (leave the webcomponent version update to the other PR though).
The generated files are the actual validation for the code generation.
For some reason it seems that it picked the new version of vaadin-overlay.
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained
a discussion (no related file):
Previously, gilberto-torrezan (Gilberto Torrezan) wrote…
Also use the code generator and push the new sources (leave the webcomponent version update to the other PR though).
The generated files are the actual validation for the code generation.
Done.
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained
a discussion (no related file):
Ok, now there are two other properties that probably are better to be excluded as well: model and owner.
There was a problem hiding this comment.
Reviewed 68 of 68 files at r4.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained
a discussion (no related file):
Previously, gilberto-torrezan (Gilberto Torrezan) wrote…
Ok, now there are two other properties that probably are better to be excluded as well:
modelandowner.
Well they are for the overlay components which we don't use anyway, but sure I can add those to ExclusionRegistry
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained
a discussion (no related file):
Previously, pekam (Pekka Maanpää) wrote…
Well they are for the overlay components which we don't use anyway, but sure I can add those to
ExclusionRegistry
Done.
There was a problem hiding this comment.
Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained
flow-components-parent/flow-generated-components/src/main/java/com/vaadin/flow/component/grid/GridVariant.java, line 26 at r5 (raw file):
"WebComponent: Vaadin.GridElement#5.1.0", "Flow#1.2-SNAPSHOT" }) public enum GridVariant { MATERIAL_COLUMN_DIVIDERS("column-dividers"), LUMO_NO_BORDER(
@gilberto-torrezan do you think this is a problem? Order changed randomly. GridVariant.values() will return in different order now.
There was a problem hiding this comment.
Reviewed 8 of 8 files at r5.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained, and 1 stale
flow-components-parent/flow-generated-components/src/main/java/com/vaadin/flow/component/grid/GridVariant.java, line 26 at r5 (raw file):
Previously, pekam (Pekka Maanpää) wrote…
@gilberto-torrezan do you think this is a problem? Order changed randomly.
GridVariant.values()will return in different order now.
I don't think it will bring any disturbance in the force, but having a deterministic order (alphabetically, for instance) would be better for sure. Better to ticketize this.
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, 1 of 1 LGTMs obtained
flow-components-parent/flow-generated-components/src/main/java/com/vaadin/flow/component/grid/GridVariant.java, line 26 at r5 (raw file):
Previously, gilberto-torrezan (Gilberto Torrezan) wrote…
I don't think it will bring any disturbance in the force, but having a deterministic order (alphabetically, for instance) would be better for sure. Better to ticketize this.
There was a problem hiding this comment.
Reviewable status:
| @@ -55,6 +58,14 @@ | |||
| excludeProperty("vaadin-dialog", "noCloseOnOutsideClick"); | |||
| excludeProperty("vaadin-list-box", "selected"); | |||
| excludeProperty("vaadin-list-box", "items"); | |||
| excludeProperty("vaadin-combo-box-overlay", "owner"); | |||
There was a problem hiding this comment.
| @@ -55,6 +58,14 @@ | |||
| excludeProperty("vaadin-dialog", "noCloseOnOutsideClick"); | |||
| excludeProperty("vaadin-list-box", "selected"); | |||
| excludeProperty("vaadin-list-box", "items"); | |||
| excludeProperty("vaadin-combo-box-overlay", "owner"); | |||
| excludeProperty("vaadin-combo-box-overlay", "model"); | |||
There was a problem hiding this comment.
|
SonarQube analysis reported 3 issues
Watch the comments in this conversation to review them. 1 extra issueNote: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
There was a problem hiding this comment.
Dismissed @vaadin-bot from 2 discussions.
Reviewable status:
This change is