PSA: Please review our coding standards before we can start coding

Software Development
,
1

Good news everyone: We’ll soon start coding Codidact. Actually, we already started working on a design framework.

However, before we can do this, we need to do one more thing: Coding standards. They need to be created, written and reviewed. The first two things have been done already, so here we go with reviewing the coding standards for the Codidact project:

There are these standard definitions currently:

These marked with Request for Comments are now subject to review. Those marked with Adopted are no more. Those marked with Work in Progress are not yet open to review.

For all marked as RfC, please do the following:

  1. Read carefully.
  2. Find things that seem to be missing, are expressed unclear, don’t seem logical to you or that you disagree with.
  3. Write an answer here with a list of these issues.

If you see an answer where you disagree with any of these 2. items, you can also reply to that answer and discuss it.

Every item will be open to review for one week, possibly a bit longer if some details aren’t resolved yet.

3

Cool.
In js guidelines what about the bool syntax?
Like is triple equals mandatory?
Can you write if(foo) { } or must you write if(foo === undefined || foo === null) { }

4

Good point. I’ve added it in: always use strict equality checking (===), unelss you’re testing for null/undefined: if (value == null).

5

I have a CSS suggestion,

When committing changes, do a fresh build of the SCSS files and commit the new outputted CSS along with the other changes.

When compiling, do both a --style compressed and a --style expanded build with the --style compressed going into a file with a .min.css extension. This makes it easier to see changes to the output and to make certain that the output isn’t changing just because of the configuration settings on your machine.

6

Re commit messages, you suggest keeping the subject bit short (70–75 characters) and I agree, but then you give an example labeled “good” that is 92 characters long.

7

The original post on this thread uses DD.MM.YYYY dates. This is confusing and hard-to-sort. Can there be a standard to put date parts in YYYY MM DD order, such as YYYY.MM.DD ?

For example, represent January 2, 2020 as 2020.01.02, not as 02.01.2020 .

6 Likes
8

@Jasper I 100% agree. I am “ugly American” but rather than impose MM/DD/YYYY on everyone else, using ISO 8601 YYYY-MM-DD is by far the best solution. Sortable. Standard for database use. Unambiguous.

15 Likes
9

My 2cts on the HTML standard:

  1. For the img tag: Please consider the alt property to be mandatory for the sake of screen readers
  2. For security purpose: Please consider enforcing the use of csp headers (or meta tag for static content not generated by a server side language)

Side note: CSP may help ensuring the JS scripts are those intended and avoid breaking the coding standard if checked by a WAF on the distribution side of things

3 Likes
10

On the committing guidelines:

  1. Single-purpose commits
    I’ve a hard time figuring how a large feature/change would behave here, does it mean changing the tests on one commit, then the code on a specific library then another commit for the usage of this library ?

    Does this mean squashing locally before pushing once all the work is done ?
    => If so I’m afraid this would frighten enthusiast helpers to even start something

  2. Should a DCO (Developer Certificate of Origin) (signed-of-by commit) be asked for ?

I’m aware a bunch of those questions are borderline between committing and contributing guidelines but feel they worth be asked for :slight_smile:

1 Like
11

To be honest, I don’t particularly mind how people prefer to commit on their own feature branches - if someone wants every message to be “some changes”, fine, as long as those changes are explained and a clear history can be kept in the PR that merges them into develop.

So perhaps we could add a preface to the committing guidelines to say that it applies only to master and develop, not to feature branches that are only worked on by one person.

3 Likes
12

I think MVP should include the selection of linters and their configuration.

13

On the JS standards:
Under Spacing - (dot point the second*) Operators
I suggest changing that point to explicitly identify binary operators and the ternary conditional, and add a point/subpoint on unary operators (++ and --), specifically that they don’t have a space between operator and operand.

*Also, I recommend numbering/naming all dot point rules for the final standard for easy future reference. Rule 5.2 or 5a is better than 'the second dot point under section 5. This recommendation applies to all the standards documents

1 Like
14

Agree on the alt attributes. Might also consider title attributes as well, which actively specify the “mouseover” popup. Should default to empty quotes.

I’ve been looking for a way to contribute to this project. CSP might be that area; as I seem to have a better-than-average understanding of how it works

3 Likes