Pull requests cleanup

Everything about development and the OpenMW source code.
User avatar
Zini
Posts: 5526
Joined: 06 Aug 2011, 15:16

Re: Pull requests cleanup

Post by Zini » 08 Aug 2018, 08:59

Most of the PRs that have piled up are flagged as Discussion, WIP or request for comments. That indicates that the author thinks these PRs aren't ready. Therefore nothing to clean up IMO.

User avatar
akortunov
Posts: 430
Joined: 13 Mar 2017, 13:49
Location: Samara, Russian Federation
Github profile: https://github.com/akortunov

Re: Pull requests cleanup

Post by akortunov » 08 Aug 2018, 09:54

Zini wrote:
08 Aug 2018, 08:59
Most of the PRs that have piled up are flagged as Discussion, WIP or request for comments. That indicates that the author thinks these PRs aren't ready. Therefore nothing to clean up IMO.
Just a start:
https://github.com/OpenMW/openmw/pull/844 - Not sure if it still needed.
https://github.com/OpenMW/openmw/pull/1600 - Android port now is a separate fork, so this PR is no more maintained.
https://github.com/OpenMW/openmw/pull/1826 - it would be nice to have your opinion here.
https://github.com/OpenMW/openmw/pull/1847 - the same thing.
https://github.com/OpenMW/openmw/pull/1063
https://github.com/OpenMW/openmw/pull/1720
https://github.com/OpenMW/openmw/pull/1808
https://github.com/OpenMW/openmw/pull/1853
https://github.com/OpenMW/openmw/pull/1852
https://github.com/OpenMW/openmw/pull/1666

User avatar
Zini
Posts: 5526
Joined: 06 Aug 2011, 15:16

Re: Pull requests cleanup

Post by Zini » 09 Aug 2018, 12:42

1852 and 1853 are already scheduled for my the next regular merge on GitHub (today). I will do a quick review for 1720 now that the WIP tag is gone.

1063 looks complex. Not sure about it. But since several devs seem to be in favour of merging it we can as well merge it and see if anything breaks. I'll add it to my queue.

1847 still has the macro issue. And maybe some others. Will get back to you on the PR itself.

1826, 1808 and 1666 are all related to animation/the NIF format. I have absolutely no clue about this stuff and definitely not the time to familiarise myself enough with it to contribute anything to the discussion (really missing scrawl in moments like this). Since the WIP/discussion tags on two of these were removed and devs had overall enough time to review them, I will add them to my merge queue.

I don't have an android device and therefore I have no way to test 1600. But yeah, probably not needed now. Should we close it?

844 sounds more like something for psi29a.

User avatar
AnyOldName3
Posts: 1118
Joined: 26 Nov 2015, 03:25

Re: Pull requests cleanup

Post by AnyOldName3 » 09 Aug 2018, 12:52

I think 844 is being kept around so that when someone else tries to do a better version in the future, they'll have access to a lot of discussion about it.
AnyOldName3, Master of Shadows

User avatar
Zini
Posts: 5526
Joined: 06 Aug 2011, 15:16

Re: Pull requests cleanup

Post by Zini » 09 Aug 2018, 12:58

Seems a bunch of PRs are missing change log entries.

@AnyOldName3 Kinda make sense. But do we need to keep the PR open for that? It doesn't go away just because it is closed. Linking it from somewhere should do the job even if it is closed (I have no idea from where though).

Post Reply

Who is online

Users browsing this forum: No registered users and 6 guests