search

asterics / WebACS

HTML5/JS version of the AsTeRICS Configuration Suite
Apache License 2.0
3 stars 2 forks source link

Sprint 201903 dynamic properties #35

Open sabicalija opened 5 years ago

sabicalija commented 5 years ago

One point is missing. When no ARE is running and WebACS is loading dynamic properties: the input field should load and display the property value, and not an empty input field instead.

klues commented 5 years ago

why a PR for merging sprint-201903-editable-properties from sprint-201903-dynamic-properties? To my mind we only need PRs if the want to merge into master.

deinhofer commented 5 years ago

why a PR for merging sprint-201903-editable-properties from sprint-201903-dynamic-properties? To my mind we only need PRs if the want to merge into master.

correct. Please change PR to merge into master

sabicalija commented 5 years ago

I did not start from master when creating this branch but from sprint-201903-editable-properties. If we change it to master now, the diff will contain changes from both, sprint-201903-editable-properties and sprint-201903-dynamic-properties.

What would be the correct process? Branch from master or is branching from other feature/bugfix branches also okay?

I can cherry-pick those two commits in a new branch, starting from master, in case we really need to change it.

klues commented 5 years ago

In the current situation there are several possibilities: (1) one PR sprint-201903-dynamic-properties into master containing all differences. Pro: easiest Con: PR's get big and not easy to overlook

(2) git checkout master git checkout -b sabic/issue#18/dynamic-properties git checkout sprint-201903-dynamic-properties -- <folder or files that are necessary for this PR> git commit ... git push origin sabic/issue#18/dynamic-properties and create a PR for this new branch sabic/issue#18/dynamic-properties into master only containing the changes that are needed for issue#18 Pro: smaller PR's, easy to overlook Con: more complex

(3) same as (2), but using cherry-pick if you know the single commits you did for solving issue#18.

For my recent 6 PR's on the AsTeRICS repository I followed approach (2).

In general a simpler appraoch would have been: a) start with master b) create new branch sabic/issue#18/dynamic-properties c) fix issue#18 d) create PR for branch sabic/issue#18/dynamic-properties into master e) checkout sprint-201903-editable-properties and call git merge master (after PR merged) or git merge sabic/issue#18/dynamic-properties (before PR merged), if you need the new changes in your working branch.

sabicalija commented 5 years ago

I did it. :smile: Thanks for the detail explanation! https://github.com/asterics/WebACS/pull/36

sabicalija commented 5 years ago

Dynamic properties are not displayed after the model is uploaded and to the ARE. Reload of page and download of the model is necessary to enable the display of the dynamic properties