Skip to content

[WIP] Basic housekeeping#5

Open
rnixx wants to merge 5 commits into
developmentfrom
housekeeping
Open

[WIP] Basic housekeeping#5
rnixx wants to merge 5 commits into
developmentfrom
housekeeping

Conversation

@rnixx

@rnixx rnixx commented Apr 28, 2020

Copy link
Copy Markdown
Member

@JohnSumskas With this PR I do basic housekeeping and try to get an overview of all your enhancements. Some examples seem to be almost duplicates of others, or behave identical. Can you tell me what the depreciated folder is good for and which examples are up to date please?

@rnixx rnixx requested a review from JohnSumskas April 28, 2020 11:51
@AndreMiras

Copy link
Copy Markdown
Member

Just curious if we do need the licence in all files or in the repo root is enough?

@JohnSumskas

Copy link
Copy Markdown

Its fine to remove the deprecated folder. It was just a list of old examples that were developed but I replaced them in the examples folder. Some of them don't work. I was just keeping them for reference. Everything outside the deprecated folder should be up to date.

…to kivy3.shaders.__init__. Cleanup stl and urdl loader examples. Remove examples/_depreciated folder. Remove all __init__.py files from examples folders. Remove license headers from source files. adopt LICENSE text.
@rnixx

rnixx commented Apr 28, 2020

Copy link
Copy Markdown
Member Author

@AndreMiras you're right. I removed the license text from py source files.

@rnixx

rnixx commented Apr 28, 2020

Copy link
Copy Markdown
Member Author

@JohnSumskas I removed the _depreciated folder from examples, fixes urdf loader example and simplifies and cleaned up urdf and stl loader examples.

@AndreMiras

Copy link
Copy Markdown
Member

Nice that you're giving it some love, it looks promising ❤️
You realise the PR is so big it makes it hard to really review? 😅
My suggestion would be, address two-three simple concerns per PR or only one concern if it's more complex. That makes it less likely to overlook something and introduce regressions.
For instance:

  • one PR that simply drops examples/_depreciated and does only that
  • one PR that only deals with license clean up
  • one PR per examples/*/main.py clean up that explains what's being done and why it's needed

It's only a suggestion and I'm not maintaining this project. It feels overkill and extra work, but actually it's a strategy to get things done. For instance even though my knowledge on this project is very limited, it's still likely that I try to understand and approve/merge something as trivial as deleting a folder called _depreciated/. So that would be one step further toward getting the big task done

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.

3 participants