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

Issue #73 - Fix contributions link in README.md #74

Merged
merged 2 commits into from
Oct 20, 2024

Conversation

diomonogatari
Copy link
Contributor

@diomonogatari diomonogatari commented Oct 16, 2024

fixes: #73

Boas,

Achei o bug #73,
Resolvi o bug.

izi pizi :D

Copy link
Contributor

@pxpm pxpm left a comment

Choose a reason for hiding this comment

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

Mete na PR description:

fixes: #73

O github faz a ligação entre o PR e a issue reportada, e assim que o PR for merged, a issue é fechada. 🥳

LGTM 👍

@diomonogatari
Copy link
Contributor Author

@pxpm done.

precisa do prefixo fixes: ?
Já tinha deixado o link para o issue :D

@pxpm
Copy link
Contributor

pxpm commented Oct 17, 2024

@pxpm done.

precisa do prefixo fixes: ? Já tinha deixado o link para o issue :D

Podes usar vários prefixos, vê aqui: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue?curius=2438

Não sei se funciona em "edits", ou se possivelmente só marca a issue se fores maintainer do repo 😢

@diomonogatari
Copy link
Contributor Author

image
olhando para o issue, parece que o milestone está porreiro.

Ty pela info

@diomonogatari
Copy link
Contributor Author

Estive a tentar perceber porque é que o Workflow falhou e ao mesmo tempo entender o que raio é um workflow (kek).

O último commit que mandei já passava no workflow, localmente (gh act).

Mas pelo meio, vi que o lint-and-test.yaml estava com errors:
image

Arranjei esta solução, corri e tive success locally again

name: Lint & test

on:
  pull_request:
    types: [opened, reopened, synchronize]

jobs:
  lint-and-test:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v3
        
      - name: Set up Node.js
        uses: actions/setup-node@v3
        with:
          node-version: '18'

      - name: Cache npm dependencies
        uses: actions/cache@v3
        with:
          path: ~/.npm
          key: ${{ runner.os }}-node-${{ hashFiles('package-lock.json') }}
          restore-keys: |
              ${{ runner.os }}-node-

      - name: Install dependencies
        run: npm ci

      - name: Concurrently lint and test
        run: npm exec concurrently npm:lint npm:test

Apesar de funcionar, é melhor validarem.
A mim parece-me fazer sentido mas é fácil escapar-me algo.

Cheers,

@pxpm
Copy link
Contributor

pxpm commented Oct 18, 2024

A mim parece-me que o "install dependencies" deve vir antes do "cache dependencies", senão o que é que vai para o cache ?

@diomonogatari
Copy link
Contributor Author

A mim parece-me que o "install dependencies" deve vir antes do "cache dependencies", senão o que é que vai para o cache ?

https://dev.to/github/caching-dependencies-to-speed-up-workflows-in-github-actions-3efl
A lógica é ires à cache 1º e assim o npm ci só vai buscar o que faltar na cache.

Alegadamente, mais uma vez estou a ver estas coisas pela 1a vez.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows
Github docs tb faz cache 1st, install next

@pxpm
Copy link
Contributor

pxpm commented Oct 18, 2024

A mim parece-me que o "install dependencies" deve vir antes do "cache dependencies", senão o que é que vai para o cache ?

https://dev.to/github/caching-dependencies-to-speed-up-workflows-in-github-actions-3efl A lógica é ires à cache 1º e assim o npm ci só vai buscar o que faltar na cache.

Alegadamente, mais uma vez estou a ver estas coisas pela 1a vez.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows Github docs tb faz cache 1st, install next

makes sense. 👍

Copy link
Member

@Alf0nso Alf0nso left a comment

Choose a reason for hiding this comment

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

Vou aprovar e dar merge, parece-me okay

@Alf0nso Alf0nso merged commit abbdc77 into devpt-org:main Oct 20, 2024
1 check passed
@pxpm pxpm mentioned this pull request Oct 24, 2024
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.

BUG - Contributing 404
3 participants