Skip to content

Conversation

@anaalves-asaas
Copy link
Contributor

@anaalves-asaas anaalves-asaas commented Jan 16, 2026

Impacto

Ajustada a lógica de verificação de prioridade P1 no CodeNarc Action para identificar corretamente se uma violação P1 está no diff. Anteriormente, o sistema bloqueava incorretamente quando havia P1s no projeto e qualquer violação (mesmo P2) no diff. Agora, com a adição de [P1], [P2] nas violationMessage dos rulesets, o bloqueio é preciso e só ocorre quando P1s reais estão nas linhas alteradas.

Link da tarefa no JIRA

Cenários testados

✅ PRs com P1s fora do diff + P2s no diff → merge permitido (antes bloqueava incorretamente)

https://github.com/asaasdev/asaas/pull/55682
https://github.com/asaasdev/asaas/actions/runs/21074707957/job/60615663760?pr=55682

✅ PRs com P1s no diff → merge bloqueado

✅ PRs sem P1s → merge permitido

@github-actions
Copy link
Contributor

🏷️ [bumpr]
Next version:2.8.0
Changes:2.7.0...asaasdev:ajustar-logica-verificacao-prioridade-p1-codenarc

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the CodeNarc Action's priority-based blocking logic by adding priority markers ([P1], [P2]) to violation messages in rulesets, enabling precise identification of P1 violations in changed lines. Previously, the system incorrectly blocked merges when P1 violations existed anywhere in the project along with any violations (even P2) in the diff. Now, blocking only occurs when actual P1 violations are detected in modified code.

Changes:

  • Added priority markers ([P1], [P2]) to all violation messages in the CodeNarc basic ruleset
  • Updated blocking logic to filter violations by priority marker before checking if they affect changed lines
  • Improved error messaging and logging for better debugging of P1 detection

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
testdata/test.groovy Added test cases demonstrating P1 and P2 violations with inline comments
testdata/basic.xml Expanded all rule definitions to include priority values and Portuguese violation messages with [P1]/[P2] markers
entrypoint.sh Enhanced check_blocking_rules() to extract and filter by priority markers, improved logging, and added safety checks for Git operations
testdata/text.md Removed test content (spelling test cases)
testdata/subdir/text.md Removed test content
Comments suppressed due to low confidence (2)

testdata/text.md:1

  • Multiple spelling errors in removed test content: 'Determinisitic' should be 'Deterministic', and 'langauge' should be 'language'.
    testdata/subdir/text.md:1
  • Corrected spelling of 'Determinisitic' to 'Deterministic'.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

entrypoint.sh Outdated
if [ "$(cat "$VIOLATIONS_FLAG")" -eq 1 ]; then
echo "P1s existem E há violações em linhas alteradas"
echo "💡 Corrija as violacoes ou use o bypass autorizado pelo coordenador."
echo "P1 encontrada em linha/arquivo alterado"
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The error message 'P1 encontrada' uses singular form, but there could be multiple P1 violations. Consider using plural 'P1s encontradas' or 'Violação(ões) P1 encontrada(s)' for accuracy.

Suggested change
echo "⛔ P1 encontrada em linha/arquivo alterado"
echo "Violação(ões) P1 encontrada(s) em linha/arquivo alterado(s)"

Copilot uses AI. Check for mistakes.
-report="$report" \
-rulesetfiles="${INPUT_RULESETFILES}" \
-basedir="." \
$includes_arg \
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

-report="$report" \
-rulesetfiles="${INPUT_RULESETFILES}" \
-basedir="." \
$includes_arg \
Copy link
Contributor

Choose a reason for hiding this comment

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

[shellcheck (suggestion)] reported by reviewdog 🐶

Suggested change
$includes_arg \
"$includes_arg" \

@anaalves-asaas anaalves-asaas marked this pull request as ready for review January 16, 2026 17:42
@anaalves-asaas anaalves-asaas marked this pull request as draft January 19, 2026 18:44
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

${INPUT_REVIEWDOG_FLAGS} || true

Copy link
Contributor

Choose a reason for hiding this comment

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

[shellcheck (suggestion)] reported by reviewdog 🐶

${INPUT_REVIEWDOG_FLAGS} || true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants