Empty lines

[ascariandrea]

I would like to propose some rules to prevent enforce rules about empty lines:

• no-multiple-empty-lines I would go for the default setting
• padded-blocks I would set: blocks => "never", classes => "always" and switches => "always"
• newline-after-var I think this rule will make the code more readable (at the expense of more lines)
• newline-before-return Maybe this is too strict, but I like it :)

@buildo/frontend what do you think??

[dhinus]

I like this. I agree on everything except switches => "always", I vote for switches => "never".

(I'm a big fan of newline-before-return :heart:)

[gabro]

Same exact feedback as @dhinus!

[ascariandrea]

This is a switch in AMS code:

switch (cellDataKey) {
    case 'confirmed': return <EventConfirmedGridCell confirmed={cellData} selected={selected} />;
    case 'severity': return <EventSeverityGridCell severity={cellData} selected={selected} />;
    case 'exportedToArchive': return <EventExportedToArchiveGridCell exportedToArchive={cellData} selected={selected} />;
    case 'note': return <EventNoteGridCell note={cellData} selected={selected} />;
    case 'eventSubjectSource': return <EventSubjectSourceGridCell eventSubjectSource={cellData} selected={selected} />;
    default: return (
      <Cell selected={selected} >
        {isADate(cellData) ? <DateTimeViewer value={cellData} /> : cellData}
      </Cell>
    );
  }

For me it's a little compressed, isn't it? 🌷

[gabro]

Option 1

switch (cellDataKey) {
    case 'confirmed': return <EventConfirmedGridCell confirmed={cellData} selected={selected} />;
    case 'severity': return <EventSeverityGridCell severity={cellData} selected={selected} />;
    case 'exportedToArchive': return <EventExportedToArchiveGridCell exportedToArchive={cellData} selected={selected} />;
    case 'note': return <EventNoteGridCell note={cellData} selected={selected} />;
    case 'eventSubjectSource': return <EventSubjectSourceGridCell eventSubjectSource={cellData} selected={selected} />;
    default: return (
      <Cell selected={selected} >
        {isADate(cellData) ? <DateTimeViewer value={cellData} /> : cellData}
      </Cell>
    );
  }

Option 2

switch (cellDataKey) {
    case 'confirmed':
      return <EventConfirmedGridCell confirmed={cellData} selected={selected} />;

    case 'severity':
      return <EventSeverityGridCell severity={cellData} selected={selected} />;

    case 'exportedToArchive':
      return <EventExportedToArchiveGridCell exportedToArchive={cellData} selected={selected} />;

    case 'note':
       return <EventNoteGridCell note={cellData} selected={selected} />;

    case 'eventSubjectSource':
       return <EventSubjectSourceGridCell eventSubjectSource={cellData} selected={selected} />;

    default:
      return (
        <Cell selected={selected} >
          {isADate(cellData) ? <DateTimeViewer value={cellData} /> : cellData}
        </Cell>
      );
  }

Option 3

switch (cellDataKey) {

    case 'confirmed':
      return <EventConfirmedGridCell confirmed={cellData} selected={selected} />;

    case 'severity':
      return <EventSeverityGridCell severity={cellData} selected={selected} />;

    case 'exportedToArchive':
      return <EventExportedToArchiveGridCell exportedToArchive={cellData} selected={selected} />;

    case 'note':
       return <EventNoteGridCell note={cellData} selected={selected} />;

    case 'eventSubjectSource':
       return <EventSubjectSourceGridCell eventSubjectSource={cellData} selected={selected} />;

    default:
      return (
        <Cell selected={selected} >
          {isADate(cellData) ? <DateTimeViewer value={cellData} /> : cellData}
        </Cell>
      );

  }

Option 4

switch (cellDataKey) {
    case 'confirmed':          return <EventConfirmedGridCell confirmed={cellData} selected={selected} />;
    case 'severity':           return <EventSeverityGridCell severity={cellData} selected={selected} />;
    case 'exportedToArchive':  return <EventExportedToArchiveGridCell exportedToArchive={cellData} selected={selected} />;
    case 'note':               return <EventNoteGridCell note={cellData} selected={selected} />;
    case 'eventSubjectSource': return <EventSubjectSourceGridCell eventSubjectSource={cellData} selected={selected} />;
    default: return (
      <Cell selected={selected} >
        {isADate(cellData) ? <DateTimeViewer value={cellData} /> : cellData}
      </Cell>
    );
  }

[gabro]

cast your vote :P

[FrancescoCioria]

But I used this :(

switch (cellDataKey) {
    case 'confirmed':
      return <EventConfirmedGridCell confirmed={cellData} selected={selected} />;
    case 'severity':
      return <EventSeverityGridCell severity={cellData} selected={selected} />;
    case 'exportedToArchive':
      return <EventExportedToArchiveGridCell exportedToArchive={cellData} selected={selected} />;
    case 'note':
       return <EventNoteGridCell note={cellData} selected={selected} />;
    case 'eventSubjectSource':
       return <EventSubjectSourceGridCell eventSubjectSource={cellData} selected={selected} />;
    default:
      return (
        <Cell selected={selected} >
          {isADate(cellData) ? <DateTimeViewer value={cellData} /> : cellData}
        </Cell>
      );
  }

[FrancescoCioria]

anyway, if I have to choose I vote for Option2

[ascariandrea]

How can we enforce the Option2 anyway? The only _option_s we can enforce are Option1 ("never") and Option3 ("always") and the only difference between Option2 and Option3 are the empty line before the first case and an empty line before the close of the switch statement.

Am I wrong?

[dhinus]

My vote is for Option2 but it's hard to enforce.

Using switches=always only enforces the empty line before the first case and after the last one. But you could still have all the cases compressed with no empty lines between them, i.e.

Option 5

switch (cellDataKey) {

    case 'confirmed': return <EventConfirmedGridCell confirmed={cellData} selected={selected} />;
    case 'severity': return <EventSeverityGridCell severity={cellData} selected={selected} />;
    case 'exportedToArchive': return <EventExportedToArchiveGridCell exportedToArchive={cellData} selected={selected} />;
    case 'note': return <EventNoteGridCell note={cellData} selected={selected} />;
    case 'eventSubjectSource': return <EventSubjectSourceGridCell eventSubjectSource={cellData} selected={selected} />;
    default: return (
      <Cell selected={selected} >
        {isADate(cellData) ? <DateTimeViewer value={cellData} /> : cellData}
      </Cell>
    );

  }

[gabro]

Yes, that's a part I was missing. The rule only enforces the lines around right? If that's the case, I would opt for "never". I care about lines in between in those cases, which it seems it can't be enforced.

[FrancescoCioria]

I say we open a different issue for this rule as it's seems it will require some time to decide a rule everyone agrees on

[ascariandrea]

Do we agree about setting "never" for switches rule?

[gabro]

Linto report for

"padded-blocks": [2, {
  "switches": "never",
  "blocks": "never",
  "classes": "always"
}]

	repo	errors	warnings
❌	buildo/aliniq	210	0
❌	buildo/web-shared	68	0
❌	buildo/react-components	41	0
❌	buildo/ipercron	52	0
✅	buildo/avenger	0	0
❌	buildo/bobafett	43	0
❌	buildo/formo	6	0
✅	buildo/hophop	0	0
✅	buildo/pledge	0	0

[gabro]

Linto report for no-multiple-empty-lines

	repo	errors	warnings
✅	buildo/aliniq	0	0
✅	buildo/web-shared	0	0
✅	buildo/react-components	0	0
✅	buildo/ipercron	0	0
✅	buildo/avenger	0	0
✅	buildo/bobafett	0	0
✅	buildo/formo	0	0
✅	buildo/hophop	0	0
✅	buildo/pledge	0	0

[gabro]

Since the last one is already ok, I've opened a PR https://github.com/buildo/eslint-config/pull/99