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

[WIP] Basic housekeeping #5

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open

[WIP] Basic housekeeping #5

wants to merge 5 commits into from

Conversation

rnixx
Copy link
Member

@rnixx rnixx commented Apr 28, 2020

@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
Member

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

@JohnSumskas
Copy link

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
Copy link
Member Author

rnixx commented Apr 28, 2020

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

@rnixx
Copy link
Member Author

rnixx commented Apr 28, 2020

@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
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