starred/LinuxGSM

Fork 0

mirror of https://github.com/GameServerManagers/LinuxGSM.git synced 2026-04-25 14:15:59 +03:00

[GH-ISSUE #1264] Code review required #979

New issue

Closed

opened 2026-02-27 02:54:39 +03:00 by kerem · 6 comments

kerem commented

2026-02-27 02:54:39 +03:00

Owner

Originally created by @dgibbs64 on GitHub (Jan 20, 2017).
Original GitHub issue: https://github.com/GameServerManagers/LinuxGSM/issues/1264

A general code review to ensure that the same standards of code are being followed across the board.

For example there are several types of if statement variations being used. These should be the same across the board

if [ "${var}" == "${var}" ]
if "${var}" -eq "${var}"
if [ "${var}" -eq 0 ]

A good idea woudl be to start a wiki page with the standards expected to ensure all developers use the same code across the board

Originally created by @dgibbs64 on GitHub (Jan 20, 2017). Original GitHub issue: https://github.com/GameServerManagers/LinuxGSM/issues/1264 A general code review to ensure that the same standards of code are being followed across the board. For example there are several types of if statement variations being used. These should be the same across the board if [ "${var}" == "${var}" ] if [[ "${var}" -eq "${var}" ]] if [ "${var}" -eq 0 ] A good idea woudl be to start a wiki page with the standards expected to ensure all developers use the same code across the board

kerem

2026-02-27 02:54:39 +03:00

closed this issue
added the
type: feature

info: docs
labels

kerem commented

2026-02-27 02:54:39 +03:00

Author

Owner

@UltimateByte commented on GitHub (Jan 21, 2017):

if "${var}" -eq "${var}"
if [ "${var}" -eq 0 ]

Those are arithmetic comparisons. For that reason, ${var} should be a numeric value, making the double quotes unnecessary.

So my question is: is this more about the look and preventing mistakes then? (Trying to understand the reason for this)

Edit: Or are you saying -eq -lt -le shouldn't be used at all, but rather == < <= ?

That's a good idea to add an entry for that onto the wiki.

Other rules i can think of atm:

Variables are lowercase regular characters only
Additional information messages are given in the form of echo " * Message here"
Any var is called with "${var}" syntax
Directories are always called using LGSM variables

Huh, there are probably plenty more of them, need to fill up a list. :o))

@UltimateByte commented on GitHub (Jan 21, 2017): if [[ "${var}" -eq "${var}" ]] if [ "${var}" -eq 0 ] Those are arithmetic comparisons. For that reason, ${var} should be a numeric value, making the double quotes unnecessary. So my question is: is this more about the look and preventing mistakes then? (Trying to understand the reason for this) Edit: Or are you saying -eq -lt -le shouldn't be used at all, but rather == < <= ? That's a good idea to add an entry for that onto the wiki. Other rules i can think of atm: - Variables are lowercase regular characters only - Additional information messages are given in the form of `echo " * Message here"` - Any var is called with `"${var}"` syntax - Directories are always called using LGSM variables Huh, there are probably plenty more of them, need to fill up a list. :o))

kerem commented

2026-02-27 02:54:39 +03:00

Author

Owner

@dgibbs64 commented on GitHub (Jan 21, 2017):

Yup this is something to build on over time. Its mainly for look and consistency to reduce errors. Something I take extra time to ensure however with there being multiple contributors I cannot always check all the code and will miss stuff somtimes.

These notes will also be useful for me to reference myself when vetting code as I myself forget which way is the standard way somtimes

@dgibbs64 commented on GitHub (Jan 21, 2017): Yup this is something to build on over time. Its mainly for look and consistency to reduce errors. Something I take extra time to ensure however with there being multiple contributors I cannot always check all the code and will miss stuff somtimes. These notes will also be useful for me to reference myself when vetting code as I myself forget which way is the standard way somtimes

kerem commented

2026-02-27 02:54:39 +03:00

Author

Owner

@UltimateByte commented on GitHub (Jan 23, 2017):

Made a base page to start with

https://github.com/GameServerManagers/LinuxGSM/wiki/Syntax-&-Conventions

@UltimateByte commented on GitHub (Jan 23, 2017): Made a base page to start with https://github.com/GameServerManagers/LinuxGSM/wiki/Syntax-&-Conventions

kerem commented

2026-02-27 02:54:39 +03:00

Author

Owner

@braunsonm commented on GitHub (Jan 26, 2017):

This can probably be closed, we have a contributing guide and have started works on a wiki page to solve this issue.
@UltimateByte @dgibbs64

@braunsonm commented on GitHub (Jan 26, 2017): This can probably be closed, we have a contributing guide and have started works on a wiki page to solve this issue. @UltimateByte @dgibbs64

kerem commented

2026-02-27 02:54:39 +03:00

Author

Owner

@UltimateByte commented on GitHub (Jan 26, 2017):

Yeah I think so, we can still improve the wiki over time though.
We got some nice doc for new contributors now already. I've seen some people coming, saying they want to learn bash, and saying they want to help with LGSM recently, which was kinda rare before, so it looks like it's having some effects already.

@UltimateByte commented on GitHub (Jan 26, 2017): Yeah I think so, we can still improve the wiki over time though. We got some nice doc for new contributors now already. I've seen some people coming, saying they want to learn bash, and saying they want to help with LGSM recently, which was kinda rare before, so it looks like it's having some effects already.

kerem commented

2026-02-27 02:54:39 +03:00

Author

Owner

@lock[bot] commented on GitHub (Jul 19, 2018):

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock[bot] commented on GitHub (Jul 19, 2018): This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.