Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: add some explanation on the properties of PathArray #3586

Merged
merged 1 commit into from Jun 9, 2020

Conversation

vocx-fc
Copy link
Contributor

@vocx-fc vocx-fc commented Jun 8, 2020

The use of App::PropertyLinkSubList for PathSubelements is a mistake because we could use a single App::PropertyLinkSub to handle both PathObject and PathSubelements.

This commit doesn't change any code, it just adds comments explaining the situation so that it isn't forgotten, and we remember to address it in the future. Ideally we should migrate the objects, but we may also decide to break compatibility with older PathArrays if both properties can't be migrated easily.

See Making the creation of arrays more user friendly.


  • Branch rebased on latest master git pull --rebase upstream master
  • Unit tests confirmed to pass by running ./bin/FreeCAD --run-test 0
  • Commit message is well-written
  • Commit message includes issue #<id> or fixes #<id> where <id> is the associated MantisBT issue id if one exists

The use of `App::PropertyLinkSubList` for `'PathSubelements'`
is a mistake because we could use a single `App::PropertyLinkSub`
to handle both `'PathObject'` and `'PathSubelements'` properties.

This commit doesn't change any code, it just adds comments
explaining the situation so that it is not forgotten,
and we remember to address it in the future.

Ideally we should migrate the objects, but we may also decide
to break compatibility with older `PathArrays` if both properties
can't be migrated easily.
@yorikvanhavre yorikvanhavre merged commit 327b306 into FreeCAD:master Jun 9, 2020
@vocx-fc vocx-fc deleted the Draft_PathArray_note branch June 9, 2020 14:13
@vocx-fc
Copy link
Contributor Author

vocx-fc commented Jun 9, 2020

@WandererFan It's not necessary to do anything at the moment, but it's just something that I'd change at some point in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants