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

Brings in vhdl style formatter. #151

Merged
merged 5 commits into from
May 14, 2024
Merged

Brings in vhdl style formatter. #151

merged 5 commits into from
May 14, 2024

Conversation

nathanaelhuffman
Copy link
Collaborator

@nathanaelhuffman nathanaelhuffman commented May 13, 2024

This can be run via buck: buck2 run //tools/multitool:multitool -- format or if you just want the report without the fixes: buck2 run //tools/multitool:multitool -- format --no-fix

I'll update the README before merging if we're happy with this interface and the application of the rules.

I'm interested in feedback on both the tool interface, and the application of the style rules

@nathanaelhuffman
Copy link
Collaborator Author

Fixes #142

Copy link
Collaborator

@Aaron-Hartwig Aaron-Hartwig left a comment

Choose a reason for hiding this comment

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

I left some comments on things I'd like to see changed. I'm not sure if they were preferences you had, or tool defaults. Happy to discuss as you see fit! These were the only things that jumped out at me in a visceral way as I scrolled through. I'm sure there will be more points to discuss as we live with this code and style. It sure is nice to look at consistently formatted files though!!

One thing I would like to understand more is the inconsistency with when there is a space between generic ( and not between generic map( (same goes for port). I have a slight preference for the space, but either way I'd just like to see us consistent in both cases.

hdl/ip/vhd/arb_mux_demux/arbiter.vhd Outdated Show resolved Hide resolved
tools/hdl.bzl Outdated Show resolved Hide resolved
tools/multitool/multitool_cli.py Outdated Show resolved Hide resolved
hdl/ip/vhd/arb_mux_demux/arbiter.vhd Outdated Show resolved Hide resolved
hdl/ip/vhd/fifos/sims/fifos_th.vhd Outdated Show resolved Hide resolved
hdl/ip/vhd/memories/dual_clock_simple_dpr.vhd Show resolved Hide resolved
Copy link
Collaborator

@Aaron-Hartwig Aaron-Hartwig left a comment

Choose a reason for hiding this comment

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

Thanks for the README updates too!

@nathanaelhuffman nathanaelhuffman merged commit b64dfbc into main May 14, 2024
2 checks passed
@nathanaelhuffman nathanaelhuffman deleted the vhdl-style branch May 14, 2024 21:10
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