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

Reprise d'API avec ScribouilliGitRepo à la place de (login, repoName) #134

Merged
merged 7 commits into from
Dec 9, 2023

Conversation

DavidBruant
Copy link
Contributor

Pfou, c'est du boulot et ça tire loin cette refonte d'API

J'ai commencé, mais c'est pas fini

J'arrête parce qu'il est tard, pas parce que c'est vain

return {
buildStatus: state.buildStatus,
theme: state.theme,
deleteRepositoryUrl: `https://github.com/${state.currentRepository.owner}/${state.currentRepository.name}/settings#danger-zone`,
deleteRepositoryUrl: `${currentRepository.publicRepositoryURL}/settings#danger-zone`,
Copy link
Contributor

@Ynote Ynote Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Peut-être que l'URL de suppression d'un repo, on peut aussi la mettre dans le ScribouilliGitRepo. Je me dis qu'elle sera probablement différente pour GitLab. T'en penses quoi ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Après réflexion, on peut peut-être le faire dans ma PR où j'ajoute les calls à GitLab, histoire de pas charger cette PR de refacto.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui, bien vu, ça devrait sortir pour être paramétrisé
Et effectivement, ça peut avoir lieu dans la PR gitlab

@DavidBruant DavidBruant changed the title début de reprise d'API avec ScribouilliGitRepo Reprise d'API avec ScribouilliGitRepo à la place de (login, repoName) Dec 5, 2023
@DavidBruant DavidBruant changed the title Reprise d'API avec ScribouilliGitRepo à la place de (login, repoName) Reprise d'API avec ScribouilliGitRepo à la place de (login, repoName) + API Gitlab Dec 5, 2023
@DavidBruant
Copy link
Contributor Author

J'ai implémenté le parcours de login avec github+gitlab comme discuté (mais pas avec les bons textes)

J'ai réussi à me login avec gitlab
Pour ça, j'ai configuré que la destination du redirect va vers localhost:8080 (et c'est dans la config toctoctoc actuellement), donc je recommande ça en local pour continuer

Je n'ai pas encore touché l'API gitlab

@Ynote
Copy link
Contributor

Ynote commented Dec 6, 2023

J'ai implémenté le parcours de login avec github+gitlab comme discuté (mais pas avec les bons textes)

Je nous mets ici le screenshot de ce qu'on a fait avec @maiwann hier :

Screenshot 2023-12-04 at 17-49-29 Scribouilli

J'ai testé cette PR en local. Idem que toi, ça marche pour la connexion GitLab. Et ça marche aussi nickel quand je retourne pour me connecter avec GitHub 👍

</ul>

<ul class="buttons">
<li><a href="./account?provider=gitlab.com" class="btn">gitlab.com</a></li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return this.callAPI(
`${gitHubApiBaseUrl}/repos/${account}/${repositoryName}`,
)
getRepository({ owner, repoName }) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'aime beaucoup ça, j'ai l'intuition que ça va nous aider si les arguments passés aux calls à l'API GitLab diffère de ceux à GitHub (par ex, l'origin de l'instance GitLab)

* @param {string} dir
* @returns
*/
async getPagesList(login, repoName, dir = '') {
async getPagesList(scribouilliGitRepo, dir = '') {
const allFiles = await this.fs.promises.readdir(
Copy link
Contributor

@Ynote Ynote Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En testant la PR, j'arrive sur un cas, après avoir supprimé une page où allFiles renvoie la page supprimée. Ce qui ensuite retourne un content à undefined sur cette ligne.

J'ai pas encore cherché où on peut régler ça. Je nous note ce bug ici en attendant.

@DavidBruant DavidBruant changed the title Reprise d'API avec ScribouilliGitRepo à la place de (login, repoName) + API Gitlab Reprise d'API avec ScribouilliGitRepo à la place de (login, repoName) Dec 7, 2023
@Ynote Ynote merged commit 8e9d17d into principale Dec 9, 2023
@Ynote Ynote deleted the git-repo branch December 9, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants