Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

follow one naming standard when naming variables #293

Closed
mfrasca opened this issue Oct 13, 2017 · 4 comments
Closed

follow one naming standard when naming variables #293

mfrasca opened this issue Oct 13, 2017 · 4 comments
Labels
documentation let's document a feature ux ux bug

Comments

@mfrasca
Copy link
Member

mfrasca commented Oct 13, 2017

Expected behaviour

single or at least consistent naming style

Actual behaviour

many variables in the new scripts/build-multiuser.nsi are all UPPERCASE (not all of them), a new file in the scripts directory starts with uppercase.

this produces an inconsistent structure, where the reader may be confused and wonder if the difference is significant. if it is not, please change back to all lowercase.

this snippet shows the situation:

!define VERSION "1.0.75" ; :bump
!define src_dir "..\dist"
!define PRODUCT_NAME "ghini.desktop"
Outfile "${PRODUCT_NAME}-${VERSION}-setup.exe"
!define PROGEXE "ghini.exe"
!define COMPANY_NAME ""
!define license_file "LICENSE"
!define readme "README.rst"
!define startmenu "$SMPROGRAMS\${PRODUCT_NAME}"
!define UNINSTALL_FILENAME "uninstall.exe"

it looks confusing, some definitions are "SHOUTING", some are quiet.

same for Add_to_PATH.vbs: it's the only file in the whole directory starting with a capital letter. traditionally, first upper case letter was meant to pull a file to the top of the listing. now less relevant, but still better follow a single naming style.

it's not a bug, really, though it may bug readers.

@mfrasca mfrasca added documentation let's document a feature ux ux bug labels Oct 13, 2017
@RoDuth
Copy link
Contributor

RoDuth commented Oct 13, 2017

see #294

and comment here

@RoDuth
Copy link
Contributor

RoDuth commented Oct 14, 2017

OK, I take that comment back, seems it will compile either way. Seems the convention is to use all capitals (a github search extension:nsi !define shows almost all are all capitals.). Changing the 4 variables I had as lowercase to all caps.

@mfrasca
Copy link
Member Author

mfrasca commented Oct 14, 2017

please follow one style on all similar files.
you added a new .nsi file, next to an existing one.
try to keep things uniform across the two files.

if the tool you rely upon insists in converting everything to all-capitals, I guess we have to adapt. if it does not, we have a choice.

if we do have a choice, you must anyway either
1- convert all existing to upper-case, or
2- convert all newly generated to lower-case.
I think (1) is easier because any subsequent addition would be in line with the existing.
I like (2) better because I don't like upper cases.
but also because it lets us use the same convention in nsi files as we already do in py files.

@mfrasca mfrasca assigned RoDuth and unassigned RoDuth Oct 15, 2017
@mfrasca
Copy link
Member Author

mfrasca commented Jun 1, 2018

closing dangling issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation let's document a feature ux ux bug
Projects
None yet
Development

No branches or pull requests

2 participants