*NEW 2015/9/4* Code submission guidelines

We sure do have a lot of rules and guidelines threads - find them all here, and please make sure you've read them! Also, community-wide announcements (that aren't major ZDoom News) go here as well.
Blzut3
 
 
Posts: 3144
Joined: Wed Nov 24, 2004 12:59 pm
Graphics Processor: ATI/AMD with Vulkan/Metal Support
Contact:

Re: *NEW 2015/9/4* Code submission guidelines

Post by Blzut3 »

Updated. I do read most GitHub comments.
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: *NEW 2015/9/4* Code submission guidelines

Post by Major Cooke »

What about two cosignatures from trusted users, and then a dev can merge without worry on medium ground changes and in territories where most familiar? I.e. decorate functions and the likes. It's understandable that much bigger and non-understood submissions such as the portal visual code, and possibly ThrustFactor require one dev at a minimum to check it out, naturally.

I just figured, "Okay, two sign-offs from two trusted users. They've done the checking for me and say it's good, so why not. Merged." <--Possibly less for the devs to have to do as Graf comes around the forums often for example and he can check the sign-offs, merge, and call it a done deal.

Also, I bring this up because there were a few times where two trusted users signed off before on submissions. I just don't recall which ones they were but one was on github and the other on the forum.
Blzut3
 
 
Posts: 3144
Joined: Wed Nov 24, 2004 12:59 pm
Graphics Processor: ATI/AMD with Vulkan/Metal Support
Contact:

Re: *NEW 2015/9/4* Code submission guidelines

Post by Blzut3 »

That's the thing, one of us has to merge it. I'm not going to blindly merge any code without verifying it first. The second+ pair of eyes is just to hopefully prevent misjudgement from destabilizing ZDoom. Like the ThrustFactor submission which broke everything but Graf probably would have merged it without the rule.
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: *NEW 2015/9/4* Code submission guidelines

Post by Major Cooke »

Question: Do we need to continuously maintain our pull requests to sort out merge conflicts all the time, or just for when huge refactors are done?
Blzut3
 
 
Posts: 3144
Joined: Wed Nov 24, 2004 12:59 pm
Graphics Processor: ATI/AMD with Vulkan/Metal Support
Contact:

Re: *NEW 2015/9/4* Code submission guidelines

Post by Blzut3 »

After huge refactors would be wise, but not a requirement in general. We can sort trivial merge issues quickly.
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: *NEW 2015/9/4* Code submission guidelines

Post by Major Cooke »

Graf Zahl wrote:Please stop making new DECORATE additions until after the release when the scriptring branch can be merged. These all will require some fixing when merged so don't expect them to be merged as-is.
Perhaps we could update the thread and title to reflect the status of new feature suggestion submissions being on temporary lockdown?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49053
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: *NEW 2015/9/4* Code submission guidelines

Post by Graf Zahl »

I don't think it's worth that, considering that you are basically the only one making such submissions and the release is quite close.
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: *NEW 2015/9/4* Code submission guidelines

Post by Major Cooke »

Eh, you never know though. ;) A_SpawnParticle came out of the blue, AND was moved from feature suggestions to code submissions, but fair enough.
User avatar
edward850
Posts: 5886
Joined: Tue Jul 19, 2005 9:06 pm
Location: New Zealand
Contact:

Re: *NEW 2015/9/4* Code submission guidelines

Post by edward850 »

I would like to point out that your actions with SpawnParticle were actually quite disruptive. I specifically asked if there were any changes anybody would think should be made, you produce no immediate issues, and then when it got merged you immediately went and changed the entire function layout causing several days of confusion.

I realise you have your own motives for all the features you add to ZDoom, but more than yourself uses this codebase. You're submitting code everyone has to work with, not just the modders (although they were just as affected in this case). How the function actually worked became useless until the devbuilds rolled for a couple of days so everyone had the same args list, and yet it still managed to creep into GZDB for awhile.
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: *NEW 2015/9/4* Code submission guidelines

Post by Major Cooke »

I actually did suggest a few things at first but they were ignored. Go look back near the end of the pages for proof. So I thought, okay, I'll just do them myself.

And here's the post to prove it.

And then the reorganizing of the function was on the request of some OTHER people such as xaser and gez to name a few.
Last edited by Major Cooke on Tue Feb 02, 2016 5:14 pm, edited 1 time in total.
User avatar
edward850
Posts: 5886
Joined: Tue Jul 19, 2005 9:06 pm
Location: New Zealand
Contact:

Re: *NEW 2015/9/4* Code submission guidelines

Post by edward850 »

You suggested exactly one thing (a physics-relative flag) and absolutely no mention of the entire order of the function, which is my main complaint here.
Major Cooke wrote:And then the reorganizing of the function was on the request of some OTHER people such as xaser and gez to name a few.
https://en.wikipedia.org/wiki/Law_of_triviality
I'm pretty sure they were bikeshedding. I would hope you know what that word means.
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: *NEW 2015/9/4* Code submission guidelines

Post by Major Cooke »

Right, but after I made a few of those changes, people asked for reorganization after the fact. They weren't particularly fond of the way it was set up. Graf said it was alright, so I did it.

I will say that was a bit annoying to deal with and I'm not planning on doing rearranging again like that.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49053
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: *NEW 2015/9/4* Code submission guidelines

Post by Graf Zahl »

@Edward:

For the record, I think the person you need to blame for the parameter shuffle is Gez who was the most vocal about changing it - before and after the original submission got merged.
User avatar
edward850
Posts: 5886
Joined: Tue Jul 19, 2005 9:06 pm
Location: New Zealand
Contact:

Re: *NEW 2015/9/4* Code submission guidelines

Post by edward850 »

He didn't seem to at the time but I wonder if it just got distracted with the bunch of other incredibly minor requests. It was one of the few odd times where the ideas for what the function was supposed to do went all over the place without any actual direction.
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: *NEW 2015/9/4* Code submission guidelines

Post by Major Cooke »

Okay, randi has now merged scripting branch within master. So that's done.

Should I start reformatting my code submissions?
Post Reply

Return to “Rules and Forum Announcements”