FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: interpret span correctly for github format by SoMuchForSubtlety · Pull Request #1621 · googleapis/api-linter · GitHub

/ api-linter Public

fix: interpret span correctly for github format - #1621

Open
SoMuchForSubtlety wants to merge 4 commits into
googleapis:mainfrom
SoMuchForSubtlety:gh-format
ConversationCommits (4)ChecksFiles changed
Open

fix: interpret span correctly for github format#1621
SoMuchForSubtlety wants to merge 4 commits into
googleapis:mainfrom
SoMuchForSubtlety:gh-format

Conversation

SoMuchForSubtlety commented Apr 9, 2026 •
edited
Loading

Copy link
Copy Markdown

The old location logic had two bugs

  • keeping locations zero-indexed
  • interpreting end column as end line in case of span size 3

I decided to reuse the existing correct implementation from the lint package. I can re-implement it if you would prefer not to expose this API.

I also fixed newlines, github does not support \n, but at least partially supports %0A, see actions/toolkit#193

Closes #1071

SoMuchForSubtlety requested a review from a team as a code owner April 9, 2026 17:55
gemini-code-assist Bot reviewed Apr 9, 2026
View reviewed changes

gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Code Review

This pull request refactors GitHub Actions output formatting by exporting and utilizing the FileLocation and Position types from the lint package. The manual span processing in github_actions.go is replaced with FileLocationFromPBLocation, and tests are updated to reflect the new output. Feedback includes suggestions to simplify formatting logic, correct a capitalization error in a comment, and improve documentation for the newly exported functions.

Comment thread cmd/api-linter/github_actions.go Outdated
Comment thread lint/problem.go Outdated
Comment thread lint/problem.go Outdated
quirogas self-requested a review April 9, 2026 21:01

quirogas commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Hey @SoMuchForSubtlety, thank you so much for submitting this PR. We definitely do not want to expose this API, so I think a small re-implementation would be appropriate. Additionally, if you don't mind, could you also reference #1071 to determine if this change addresses that issue?

fix: interpret span correctly for github format
4951dc6 
Reuse location logic from the lint package

The old location logic had two bugs
- keeping locations zero-indexed
- interpreting end column as end line in case of span size 3
fix docstrings
99a08f5
revert API changes
7c46e11
re-implement
d4be20d

SoMuchForSubtlety commented Jun 18, 2026

Copy link
Copy Markdown
Author

@quirogas any chance of getting another review?

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

quirogas Awaiting requested review from quirogas
1 more reviewer
gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

GitHub Action output format generates incorrect line numbers

2 participants

Back | FazBrowse Home | New Git URL