Skip to content

Conversation

@BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Jun 4, 2019

Changes:

  • Remove empty builder tool
  • Remove old manager and loader, replace with cbloader and cbsceneinventory
  • Refactor cbloader -> loader
  • Refactor cbsceneinventory -> sceneinventory

Reference issue: #364

NOTE
Currently code is untested, only refactored it for now. This way it also allows others to check their dependencies on it plus potentially discuss the naming/refactoring of "sceneinventory" as opposed to the older "manager".

BigRoy added 2 commits June 4, 2019 09:44
- Remove old manager and loader, replace with cbloader and cbsceneinventory
- Refactor cbloader -> loader
- Refactor cbsceneinventory -> sceneinventory
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-267: W605 invalid escape sequence '\M'
./avalon/maya/compat.py:25:-262: W605 invalid escape sequence '\s'
./avalon/maya/compat.py:25:-254: W605 invalid escape sequence '\g'
./avalon/maya/compat.py:25:-238: W605 invalid escape sequence '\h'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-267: W605 invalid escape sequence '\M'
./avalon/maya/compat.py:25:-262: W605 invalid escape sequence '\s'
./avalon/maya/compat.py:25:-254: W605 invalid escape sequence '\g'
./avalon/maya/compat.py:25:-238: W605 invalid escape sequence '\h'

@mottosso
Copy link
Contributor

mottosso commented Jun 4, 2019

Looks good, this is the right way forwards.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 4, 2019

@mottosso do you happen to know what kind of drugs the Hound sniffed? :) I'm assuming those lines are {file}.py:{line}:{column}? Is it referring to negative columns/characters? Wut?

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-267: W605 invalid escape sequence '\M'
./avalon/maya/compat.py:25:-262: W605 invalid escape sequence '\s'
./avalon/maya/compat.py:25:-254: W605 invalid escape sequence '\g'
./avalon/maya/compat.py:25:-238: W605 invalid escape sequence '\h'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-267: W605 invalid escape sequence '\M'
./avalon/maya/compat.py:25:-262: W605 invalid escape sequence '\s'
./avalon/maya/compat.py:25:-254: W605 invalid escape sequence '\g'
./avalon/maya/compat.py:25:-238: W605 invalid escape sequence '\h'

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-276: W605 invalid escape sequence '\p'
./avalon/maya/compat.py:25:-267: W605 invalid escape sequence '\M'
./avalon/maya/compat.py:25:-262: W605 invalid escape sequence '\s'
./avalon/maya/compat.py:25:-254: W605 invalid escape sequence '\g'
./avalon/maya/compat.py:25:-238: W605 invalid escape sequence '\h'

@mottosso
Copy link
Contributor

mottosso commented Jun 4, 2019

@mottosso do you happen to know what kind of drugs the Hound sniffed? :)

Haha, no idea. Could it be line-endings, Windows versus Linux? Didn't you have a new file-server, could that have had an effect?

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 4, 2019

Haha, no idea. Could it be line-endings, Windows versus Linux? Didn't you have a new file-server, could that have had an effect?

Everything is saved from a Windows workstations. Plus it's the only file showing the odd behavior and it doesn't even show any other changes than one change on line 97. Additionally, checking symbols in the file doesn't show me anything odd other than just line endings everywhere. It's really finding something magically hidden.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 4, 2019

It seems it failed on the backslashes in the docstring. Seems like a bug, anyway... changed it with e2f21d3 and it seems to make it pass. :)

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 4, 2019

@davidlatwe @tokejepsen @mkolar - tagging you guys to give this a test run - if you're referencing any of the tools in your own pipeline code note that they might need refactoring with these names changes. As such, if you are using these tools that are NOT public in Avalon API this could introduce a backwards incompatibility and will require you to update code along.

That is also described in this PR's issue #388

Also, AVALON_EARLY_ADOPTERS is effectively redundant with this change. It does not trigger any changes on behavior, but could still be used in the future if we introduce breaking changes in a point release. So note that the key is still mentioned here


This merge will likely become part of Avalon 6.0 - see: https://github.com/getavalon/core/projects/3

@davidlatwe
Copy link
Collaborator

I have tested on my end and looks working nicely after I changed those module name :)
And I also made a PR in Launcher getavalon/launcher#42 for this to working.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jun 11, 2019

And I also made a PR in Launcher getavalon/launcher#42 for this to working.

Thanks, totally forgot that action was native to the launcher currently.

davidlatwe added a commit to MoonShineVFX/avalon-core that referenced this pull request Jul 10, 2019
davidlatwe added a commit to MoonShineVFX/avalon-core that referenced this pull request Jul 10, 2019
@mkolar
Copy link
Member

mkolar commented Aug 6, 2019

This does not affect us much. We're actually preparing (and should be done this week @iLLiCiTiT? ) a refactor of the UIs which separates models, delegates and widgets from their tools into universally usable elements for UI building in avalon. As we worked with the tools it became a bit of a mishmash of tools cross-referencing widgets from other tools, so this is an effort to make it cleaner... #upcoming

@BigRoy
Copy link
Collaborator Author

BigRoy commented Aug 6, 2019

a refactor of the UIs which separates models, delegates and widgets from their tools into universally usable elements for UI building in avalon. As we worked with the tools it became a bit of a mishmash of tools cross-referencing widgets from other tools, so this is an effort to make it cleaner... #upcoming

Perfect! :)

@mottosso mottosso mentioned this pull request Aug 20, 2019
@mottosso mottosso merged commit e2f21d3 into getavalon:master Aug 20, 2019
mottosso added a commit to mottosso/avalon-core that referenced this pull request Aug 21, 2019
mottosso added a commit that referenced this pull request Aug 21, 2019
Repair broken loader, since #388
@mottosso mottosso mentioned this pull request Aug 21, 2019
@BigRoy BigRoy deleted the fix364 branch January 25, 2021 14:56
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.

5 participants