Skip to content

Linter action with LibreCores CI#960

Open
wallento wants to merge 1 commit intolowRISC:masterfrom
wallento:lint-action
Open

Linter action with LibreCores CI#960
wallento wants to merge 1 commit intolowRISC:masterfrom
wallento:lint-action

Conversation

@wallento
Copy link
Member

This adds a beta Github action that is run on pushes and pull
requests. It automatically annotates the sources with linter issues.

Nothing extra is needed beyond merging.

As a demo you can see how new PRs will fail: wallento#4
They will then show the annotation: wallento@bbf8e10

Removing the current waiver looks like this as a test: wallento@966649b
So, you can see that it will also annotate files outside the changeset.

There will be robustness extensions, but I think it is usable and I would love ibex as beta testers. It can be deactivated or removed as easily as it was added :)

@wallento
Copy link
Member Author

Links are broken by github autolinking, the second one is accessible via wallento#5

This adds a beta Github action that is run on pushes and pull
requests. It automatically annotates the sources with linter issues.

Nothing extra is needed beyond merging.

Signed-off-by: Stefan Wallentowitz <stefan.wallentowitz@hm.edu>
Copy link
Contributor

@imphil imphil left a comment

Choose a reason for hiding this comment

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

Uh, turns out I completely forgot about this; sorry @wallento!

To clarify, this is using Verilator as linter and posts comments as code annotations, while #1427 uses Verible lint and posts lint output as comments (which we can then reply to).

Effectively the two PRs do two different things in two different ways. Since it is not really clear a the moment which approach to present linter output to the user is better, I'm happy to give both approaches a try and we can then improve as we move along.

@@ -0,0 +1,25 @@
on: [push, pull_request]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a license header here?

jobs:
lint:
runs-on: ubuntu-latest
name: Linter
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: Linter
name: RTL lint with Verilator

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.

2 participants