Ajout de nouvelles actions qui clarifie les opérations pour modifier ou ajouter un fichier #110
Conversation
|
@DavidBruant J'ai testé ces différents scénarii à la mano en local et ils fonctionnent :
Je fais les modifications sur la PR #109 et je te laisse me faire une relecture 💜 |
|
@DavidBruant La PR a des conflits car j'ai fait les changements sur #109. Je vais reprendre dessus dans l'aprem. Si tu n'as pas commencé de review, je te propose de te reping quand cette PR est plus clean :) |
…new-writefile-action * 'principale' of github.com:Scribouilli/scribouilli: Ajout de nouvelles actions qui clarifie les opérations pour supprimer un fichier (#109)
Ynote
force-pushed
the
new-writefile-action
branch
from
438925e
to
12c3f98
Compare
|
@DavidBruant C'est ok de mon côté, j'ai fixé les conflits et fait quelques mini-changements. J'ai retesté en local, les cas que j'ai listés ici fonctionnent. Je veux bien une relecture dessus quand tu as du temps :) |
| store.state.currentRepository.owner, | ||
| store.state.currentRepository.name, | ||
| 'Changement index', | ||
| ) |
There was a problem hiding this comment.
J'adore ! Et ça marche ? Genre, ça créé un commit unique plus propre ?
There was a problem hiding this comment.
Ouiiiiii ! Par exemple ici
| const { owner, name } = state.currentRepository | ||
| export const writeFileAndCommit = (fileName, content, commitMessage = '') => { | ||
| if (commitMessage === '') { | ||
| commitMessage = `Modification du fichier ${fileName}` |
There was a problem hiding this comment.
Si y'a cette ligne, ça ne sert à rien de mettre la valeur par défaut dans la signature de la fonction
There was a problem hiding this comment.
Ou ça pourrait être direct la valeur par défaut
There was a problem hiding this comment.
Je ne mets pas la valeur par défaut directement parce que j'ai envie qu'on ait le nom de fichier dans le commit par défaut.
Par contre, quand je retire la valeur par défaut, et en changeant la condition avec
if (typeof commitMessage !== 'string' || commitMessage === '') {Ça me renvoie cette erreur parce que databaseAPI.commit prend un argument message de type string :
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.
J'ai essayé de changer la condition de plein de manière différente, l'erreur est toujours lancée :/
There was a problem hiding this comment.
Ça m'embête que le check continue de lancer une erreur, j'ai mis un @ts-ignore et mis à jour la méthode avec la condition if (typeof commitMessage !== 'string' || commitMessage === '').
| @@ -118,13 +119,10 @@ | |||
| for (const img of image) { | |||
| imageMd = 'Mise en ligne en cours…' | |||
| const buffer = new Uint8Array(await img.arrayBuffer()) | |||
| await databaseAPI.writeFile( | |||
| currentRepository.owner, | |||
| currentRepository.name, | |||
There was a problem hiding this comment.
ah ouais, c'est cool de ne plus avoir ces paramètres ici et de les avoir plutôt en interne dans l'action !
|
C'est ok et testé en prod avec les cas listés ici. |
…add-tests * 'principale' of github.com:Scribouilli/scribouilli: Ajout de nouvelles actions qui clarifie les opérations pour modifier ou ajouter un fichier (#110)
Lié à #109
Description
Actuellement en prod, la modification ou la création d'un fichier se fait avec une seule méthode DatabaseAPI.writeFile qui fait plusieurs opérations en même temps :
Comme pour #109, cette manière de faire crée des patterns bizarres et peu flexibles dans le code.
J'ai choisi de :
commitetpushde la méthodeDatabaseAPI.writeFile,DatabaseAPI.writeFile,actions/file.jsactions/page.jset celles des articles dansactions/article.js,DatabaseAPI.writeFile.Le fait d'ajouter ces actions clarifie mieux comment se passe les opérations mais ajoute un peu de redondance. J'ai l'impression que c'est ok d'avoir quelques répétitions car elles permettent de plus facilement lire le code.