Initial commit of m64py
Details
Details
- Reviewers
JoshStrobl DataDrake - Group Reviewers
Triage Team - Maniphest Tasks
- T4012: M64Py
- Commits
- R4530:d6809e44d03c: Inital commit of m64py
Open m64py and load the test ROM from their repository: https://github.com/mupen64plus/mupen64plus-ui-python/raw/master/test/mupen64plus.v64
Diff Detail
Diff Detail
- Branch
- master
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| package.yml | ||
|---|---|---|
| 7 | You're missing a lot of other licenses: https://github.com/mupen64plus/mupen64plus-ui-python/blob/master/LICENSES | |
| 16–18 | Should be alphabetically sorted. | |
| 19–20 | Should be removed. | |
Comment Actions
Thank you for the review! I changed everything as requested, though I am still unsure about the licenses part. Is it okay like this now, or if it is not, could you maybe point me to another example of a package with multiple licenses like this?
Comment Actions
Sorry for the delay in the review.
| package.yml | ||
|---|---|---|
| 8–14 | All the () should be behind # for commenting, otherwise they'll show up in the spec which is undesired. | |
Comment Actions
Thank you again for the review. I don't mind the delay, since I know you've been busy (and m64py isn't, and shouldn't, be too high on the Solus priority list ^^).
I've changed the licenses part accordingly. No idea why I didn't even think about commenting out that part myself :)
Comment Actions
I would prefer having this built against python 3, python 2 will be retired in a year so I'd suggest to always prefer python 3 for new inclusion in the repository unless there is a blocker.
| package.yml | ||
|---|---|---|
| 13 | You should not put 3 times the same license. Instead do something like this Also you should alphabetize the list | |
Comment Actions
Thank you for the input, kyrios123. I've changed everything accordingly; totally forgot about using python3, even though I did so with another package I built for myself.