[FEATURE] Ajout d'une règle forcant l'ajout d'un commentaire sur les colonnes.

[VincentHardouin]

Reouverture de #25

🦄 Problème

Dans le but de faciliter la compréhension des nos modèles de données, il est préférable d'ajouter une description à nos colonnes. Knex permet de le faire grâce à la fonction comment(). Afin d'éviter les oublis, nous avons décider d'ajouter une règle de lint qui empêche l'ajout ou la modification d'une colonne sans ajouter de commentaire.

🤖 Proposition

Ajouter une règle de lint forçant l'ajout d'un commentaire.

🌈 Remarques

Il faudra désactiver la règle sur les anciennes migration (npx suppress-eslint-errors@2.0.4 . --rules=@1024pix/no-column-migration-without-comment)

💯 Pour tester

Vérifier que les test passent (la règle a été testée sur le code du mono repo).

[Steph0]

Test func sur la branche add-columns-comment de Pix et en faisant un npm install /home/stephen/Pix/eslint-plugin depuis cette branche.

J'ai pas mal de test qui sont KO, je me dis que je teste peut-être mal. Est-ce possible de bien remplir la section "à tester" de cette PR ?

Je mets quand même mes tests ci-dessous

Essai avec colonne sans comment ⚠️

Avec ce up, le linter ne se déclenche pas

const TABLE_NAME = 'schools';
const COLUMN_NAME = 'testExpirationDate';

const up = async function (knex) {
  await knex.schema.table(TABLE_NAME, function (table) {
    table.dateTime(COLUMN_NAME).defaultTo(null);
  });
};

Avec ce up ci oui

const TABLE_NAME = 'certification-candidates';
const COLUMN_NAME = 'hasSeenCertificationInstructions';

const up = function (knex) {
return knex.schema.table(TABLE_NAME, (table) => {
  table.boolean(COLUMN_NAME).defaultTo(false);
});
};

Commentaire à vide ⚠️

Un commentaire vide passe le linter

const TABLE_NAME = 'certification-candidates';
const COLUMN_NAME = 'hasSeenCertificationInstructions';

const up = function (knex) {
return knex.schema.table(TABLE_NAME, (table) => {
  table.boolean(COLUMN_NAME).defaultTo(false).comment('');
});
};

Un autre exemple où je m'attendrais à avoir 'name' qui déclenche le linter, tiré de la documentation de knex

const up = function (knex) {
  return knex.schema.createTable('users', function (table) {
    table.increments('userId');
    table.string('name');
  });
};

Primary key ✔️

Pas de détection si primary key

const up = function (knex) {
knex.schema.createTable('test', function (table) {
  table.primary('email', {
    constraintName: 'users_primary_key',
    deferrable: 'deferred',
  });
});
};

[Steph0]

Je maitrise pas trop es-lint, mais ici du coup ce sont les instructions de knex autorisées (non lintées) c'est cela ?

Je propose d'ajouter primary

Eventuellement, pourquoi ne pas inverser la règle, et mettre une blacklist ? Car j'ai l'impression que l'on pourrait mettre beaucoup + dans la whitelist (par exemple dropNullable, droptimestamps, etc.), mais je loupe peut-être un truc dans mon chemin de pensée

[Steph0]

Curiosité, je comprenais pas dans le message pour 'table.column' sur ma migration qui contient une 'table.string('name');' ; est-ce que le linter pourrait remonter la ligne fautive (le numéro, ou l'instruction) ?