search

openhwgroup / core-v-mcu

This is the CORE-V MCU project, hosting CORE-V's embedded-class cores.
https://docs.openhwgroup.org/projects/core-v-mcu
Other
158 stars 50 forks source link

Latest (v0.0-3373-g54e37ed0) verison of Verible #319

Closed MikeOpenHWGroup closed 10 months ago

MikeOpenHWGroup commented 11 months ago

The latest update to verible is changing the formatting of a lot of MCU files. Please do not merge this unless/until theDo Not Merge label is removed.

MikeOpenHWGroup commented 11 months ago

@zarubaf, @gmartin102

Following Florian's suggestion, I updated the version of Verible I use on my local machine and in the GitHub Actions of this repo to the latest pre-built version for Ubuntu 20.04 (v0.0-3373-g54e37ed0). The good news is that this seems to work and now all Lint checks are passing.

The not-so-good news is that the latest version of Verible has significantly (?) different formatting rules than the current version we are using (v0.0-1051-gd4cd328) and 30 SystemVerilog files have been re-formatted. I have little doubt that the new formatting is "better", but do we want to change that many RTL files given that we should be working to create a tag with exactly what we released to CMC.

zarubaf commented 11 months ago

I think in principle that change is good and necessary as the tool evolves, so I would vote it in.

If the goal is just to produce the RTL Freeze state then obviously this isn't as good and we should probably waive the verible check for the failing commits. Agree?

MikeOpenHWGroup commented 11 months ago

If the goal is just to produce the RTL Freeze state then obviously this isn't as good and we should probably waive the verible check for the failing commits. Agree?

Agreed. So here is what I propose:

  1. Waive the Verible checks for #314 from @gmartin102. This will allow us to create a v1.0.0 for CORE-V-MCU that will match what we actually built.
  2. After creating the v1.0.0 release, merge in this PR (or a version thereof) and that will start the ball rolling on the next release (not yet planned).
zarubaf commented 11 months ago

Agree, we can merge with the CI failing.

MikeOpenHWGroup commented 10 months ago

I managed to find the secret sauce to get the verible CI checks to pass. See the README for details.

Merging it in...