dearmochi

PR Reviewer Application - dearmochi

Recommended Posts

Posted (edited)

Forum username: dearmochi

Discord (include tag numbers): mochi#5162

CKEY (BYOND username): Shendow

Characters you play in-game: Jade Styles

Link to the list of PRs you've done: Full list

I'll link the PRs I think are the most relevant for this application - code quality, scope etc. Not all of them are merged yet. Some are pending review but I am confident they are adequate in quality.

Link to the list of PR reviews you've done: Full list

Link to any other examples of your PR work (e.g. on other codebases) to give us an idea for your skill as a PR author:

This is the first open-source project I contributed PRs to, but I do have professional experience when it comes to code reviews so I have knowledge of how the process works.

Link to any other examples of your review work (e.g: on other codebases) to give us an idea of your abilities as a PR reviewer:

Likewise.

What do you think the most common issue is for PRs submitted to Paradise, and why?

From what I've seen, a big issue I've seen is documentation and clarity for future coders. Code is often written in a way that it's hard to understand for coders other than the PR author (due to the lack of comments OR readability). Since this is an open-source project I believe any new code should be written such that modifying or adding to it doesn't necessitate extensive amounts of time to understand what it does and how it works.

If you could make one single change to our coding guidelines, what would it be?

Adding to what I've said above, code clarity is important. This includes multiple methods: good code design (proc separation, etc.), sensible naming for procs, vars and defines, short comments explaining the intent or shedding light on obscure parts and most of all: consistency. Granted, the codebase contains stuff that is over 8 years old and it will probably be quite some time before it's brought up to the ever-changing standards, but that doesn't mean new code can't be clean - on the contrary!

Edited by dearmochi
  • Like 1
  • fastparrot 1

Share this post


Link to post
Share on other sites
On 7/14/2020 at 2:26 PM, dearmochi said:

Forum username: dearmochi

Discord (include tag numbers): mochi#5162

CKEY (BYOND username): Shendow

Characters you play in-game: Jade Styles

Link to the list of PRs you've done: Full list

I'll link the PRs I think are the most relevant for this application - code quality, scope etc. Not all of them are merged yet. Some are pending review but I am confident they are adequate in quality.

Link to the list of PR reviews you've done: Full list

Link to any other examples of your PR work (e.g. on other codebases) to give us an idea for your skill as a PR author:

This is the first open-source project I contributed PRs to, but I do have professional experience when it comes to code reviews so I have knowledge of how the process works.

Link to any other examples of your review work (e.g: on other codebases) to give us an idea of your abilities as a PR reviewer:

Likewise.

What do you think the most common issue is for PRs submitted to Paradise, and why?

From what I've seen, a big issue I've seen is documentation and clarity for future coders. Code is often written in a way that it's hard to understand for coders other than the PR author (due to the lack of comments OR readability). Since this is an open-source project I believe any new code should be written such that modifying or adding to it doesn't necessitate extensive amounts of time to understand what it does and how it works.

If you could make one single change to our coding guidelines, what would it be?

Adding to what I've said above, code clarity is important. This includes multiple methods: good code design (proc separation, etc.), sensible naming for procs, vars and defines, short comments explaining the intent or shedding light on obscure parts and most of all: consistency. Granted, the codebase contains stuff that is over 8 years old and it will probably be quite some time before it's brought up to the ever-changing standards, but that doesn't mean new code can't be clean - on the contrary!

Sorry, but after a head+maint vote we must decline this app.

The primary reason is that you're still very new contributing to paradise. Your first PR was https://github.com/ParadiseSS13/Paradise/pull/13588 which was opened Jun 9th, or less than two months ago! Most of the current coding staff have been contributing for years.

The secondary reason is that you recently had a PR which, if merged, would have caused database issues. Thankfully AA caught it, but since the primary job of a PR reviewer is to spot issues with code, the fact that you did not test/catch this yourself was worrying.

My suggestion is that you keep contributing for another 6 months or so, then reapply. With a longer, good track record behind you, your second app should have a higher chance of success.

Share this post


Link to post
Share on other sites