VUnit / vunit

VUnit is a unit testing framework for VHDL/SystemVerilog
http://vunit.github.io/
Other
736 stars 263 forks source link

VHDL 'package name' and 'file name' inconsistencies need to be fixed #760

Open dpaul24 opened 3 years ago

dpaul24 commented 3 years ago

Some package files under **https://github.com/VUnit/vunit/tree/master/vunit/vhdl/*** have inconsistencies in their package name and file name. So there is a need to modify the file names of such VHDL packages.

The following a list which needs modification.

image

Adding more...

  1. Package files under https://github.com/VUnit/vunit/tree/master/vunit/vhdl/com/src/* not having the suffix _pkg need to be renamed.

  2. VUnit/vunit/blob/master/vunit/vhdl/string_ops/src/string_ops.vhd is defined as a package but the _pkg is missing, both in the package definition and file name.

  3. VUnit/vunit/blob/master/vunit/vhdl/dictionary/src/dictionary.vhd is defined as a package but the _pkg is missing, both in the package definition and file name.

  4. VUnit/vunit/blob/master/vunit/vhdl/path/src/path.vhd is defined as a package but the _pkg is missing, both in the package definition and file name.

  5. Files under VUnit/vunit/tree/master/vunit/vhdl/run/src/* not having the suffix _pkg need to be renamed.

std-max commented 2 years ago

I was browsing through packages and notice the same issue, nice to see it is already opened 😉 I've some questions:

Last question:

umarcor commented 2 years ago
  • What does the p mean after 2008 or 2002 in a package name ? Is it like "for 2002 (or 2008) and newer standards ?

Yes. It means "2002 or newer", "2008 or newer" or "2019 or newer".

  • In order to increase consistency, I know that could increase a bit the number of files but what do you think of having one file for package and one file for package body for each package ? I think for packages like string_ops where the file is around 700 lines, it could be easier to read their specification. For packages like dictionnary (short one), it might be less obvious but I'm on the side of consistency. What do you think?

Agree.

  • When do you think we should make the changes indicated by @dpaul24 ? For v4.7.0 ? v5.0 ?

Since it's a breaking change for consistency purposes only, it should be for v5.

LarsAsplund commented 2 years ago

@umarcor As I understand these are just file name changes. How is that a breaking change? Are you thinking third-party build scripts?

umarcor commented 2 years ago

@LarsAsplund yes. We might consider it a not-so-relevant breaking change. However, since we have other breaking changes in the queue, and it's been half a year since the last release, I think we might publish v4.7.0 in May without braking changes and then start merging towards v5. What do you think?

LarsAsplund commented 2 years ago

@umarcor My mistake, there are breaking changes. For example, changing dictionary to dictionary_pkg. OTOH, I'm not sure there is enough value in such a change to motivate the break, even if it is done along with other breaking changes. I think most people are using the contexts these days and do not even see the package name. Either way, a 4.7 release without breaking changes is a good start.