Closed urbite closed 2 weeks ago
Hi @urbite.
Thanks for raising this issue.
I'll wait for your confirmation that the format of this issue is acceptable before creating additional issues.
The formatting of this issue is perfect.
How did you get the highlighted others in your title.
This was just by using a single backtick on either side of the keyword, i.e.
Rules to enforce case on the `others` keyword in selected assignments
I'll aim to raise a PR for this issue soon. I haven't used this construct myself, so I'm not sure whether the parser handles it well. Assuming that it does handle the construct well, this should be a very easy rule to implement.
Update: the parser does recognise these tokens already, so it should be straightforward to implement this rule. I should be able to do so soon.
12 | type flag_pt is PROTECTED body
<class 'vsg.token.full_type_declaration.type_keyword'>
<class 'vsg.token.full_type_declaration.identifier'>
<class 'vsg.token.full_type_declaration.is_keyword'>
<class 'vsg.token.protected_type_body.protected_keyword'>
<class 'vsg.token.protected_type_body.body_keyword'>
Thanks Jukka
While I'm working on the protected
keyword, I also plan to add rules to enforce case for the body
and end
keywords as well.
package test is
type flag_pt is PROTECTED
END PROTECTED;
end package test;
package body test is
type flag_pt is PROTECTED BODY
END PROTECTED BODY;
end package body test;
While I'm working on the
protected
keyword, I also plan to add rules to enforce case for thebody
, andend
keywords as well.
I did notice that the vsg version that I built didn't fix parameter directions, which appear to have been merged in commit 2c6019. To build from the latest repo commit, do I need to check out that commit before building? If building after a clone or pull (without any checkout), is the latest tagged commit what is built?
It appears that the latest commit is what I had checked out when I built. But changes from prior commits don't appear to be functional in the resulting vsg build. I'm sure there's a conceptual error here on my part.
>git rev-parse --short HEAD
dd647776
Is there any vsg option (possibly hidden) to get the git hash that was used for the build. I only see the -v (version) option, which shows the following. If not available, this might a nice thing to have. :smile:
(venv) C:\projects\VHDL\vhdl-style-guide>python bin\vsg -v
VHDL Style Guide (VSG) version: 3.27.0
After re-reading the docs on Development Environment, I realized that building is only required for generating installation artifacts. https://vhdl-style-guide.readthedocs.io/en/latest/developing/development_environment.html
To use the latest cloned and checkout version doesn't require any building - just invoke vsg from the bin directory. Set an alias to make this even easier.
$ set alias vsg='python3 <path_to_clone_directory>/bin/vsg'
@urbite
Should it take this long?
I'm not sure, I don't build VSG locally, I just use the scripts directly by running bin/vsg
when I'm testing. It's possible that your method for building is also running all of the unittests, in which case, I'm not surprised that it takes a while.
To build from the latest repo commit, do I need to check out that commit before building? If building after a clone or pull (without any checkout), is the latest tagged commit what is built?
Again, I don't build VSG locally, but I can make some educated guesses. If you want to build the latest commit, you should be on the master branch and you should do a pull before you run the build. If you're building locally, it should build the commit that you're currently on.
A more general git(hub) flow question: Does a commit not show up until after a PR is accepted and merged?
Are you familiar with the concept of branches in git? If not, the idea is that there is a "master branch", and development work is done on separate branches which split off from the master branch. When a PR is raised, it is a request for a development branch to be merged (or "pulled", hence the name Pull Request) back into the master branch. Code that is on a development branch will not show up on the master branch until the development branch's PR has been approved and merged. If you want to access that code prior to the merge, you need to "check out" the development branch, e.g. git checkout <branch name>
. I hope that makes sense.
I only see the -v (version) option, which shows the following.
I'm not sure what's going on there. On my clone, which is at the head of the master branch, I get
$ bin/vsg -v
VHDL Style Guide (VSG) version: DEVELOP
$ git rev-parse HEAD
dd647776503a1a4bb0f4be6c844b8394e38dc106
If you use VSG without building, and invoke it from the bin directory as you mentioned above, hopefully you will have more success. Python is a dynamic language, rather than a static language like C, so you don't need to "build" Python source code to run it.
I discovered why running vsg with a clone of the latest repo wasn't work. Apparently it was because I had already installed vsg via pip. Even though the path to the latest vsg was given, for some reason the release version of vsg was being used.
(venv) C:\projects\VHDL\vhdl-style-guide>python bin/vsg -v
VHDL Style Guide (VSG) version: 3.27.0
(venv) C:\projects\VHDL\vhdl-style-guide>git rev-parse HEAD
dd647776503a1a4bb0f4be6c844b8394e38dc106
Removed the vsg module, now the vsg develop version is shown.
(venv) C:\projects\VHDL\vhdl-style-guide>python -m pip uninstall vsg
Found existing installation: vsg 3.27.0
Uninstalling vsg-3.27.0:
Would remove:
c:\projects\vhdl\venv\lib\site-packages\vsg-3.27.0.dist-info\*
c:\projects\vhdl\venv\lib\site-packages\vsg\*
c:\projects\vhdl\venv\scripts\vsg.exe
Proceed (Y/n)? y
Successfully uninstalled vsg-3.27.0
(venv) C:\projects\VHDL\vhdl-style-guide>python bin/vsg -v
VHDL Style Guide (VSG) version: DEVELOP
(venv) C:\projects\VHDL\vhdl-style-guide>git rev-parse master
dd647776503a1a4bb0f4be6c844b8394e38dc106
Running the DEVELOP version of vsg from the repo HEAD worked great - all of the keywords addressed by the merged PRs were changed! Thanks!!!
Building VSG Knowing that python is interpreted, I was a little puzzled that a 'build' was needed to run vsg. My understanding now (please confirm) is that the build is for creating the wheel file to allow a module to be installed via pip. This could be useful if one wanted to distribute the latest version of vsg to other developers on a team. Then the team members could pip install from the wheel file and not have to clone the vsg repo. Newer versions of the wheel file could be built and distributed as vsg improvements are made.
PRs and branches I was a bit brain-dead and didn't word things well in my earlier post. I have some limited experience in the git flow, in both submitting PRs and approving/merging them - but it's been while. I'm a bit puzzled because I see your commits, but don't see any branches. And I don't see the vsg forked repo in your repo list. I understood the process for contributing to an open source project as:
Here's a project I contributed to earlier this year that followed this process. https://github.com/urbite/micropython_rotary_encoder
Please pardon my git naivete 🙂
@urbite
I discovered why running vsg with a clone of the latest repo wasn't work
Ah I see, I'm glad that you solved the problem!
My understanding now (please confirm) is that the build is for creating the wheel file to allow a module to be installed via pip.
Yes, I don't know a great deal about the distribution of python packages, but that's my understanding as well. The tagged builds are released to be installed through pip, which is the intended way for the end users to use VSG https://pypi.org/project/vsg/
I understood the process for contributing to an open source project as:
Ah sorry, sounds like I misunderstood the question completely 😅 Yes, your understanding is correct, that is the process that I follow as well. My fork of the repository can be found here, along with the branches that I've created https://github.com/JHertz5/vhdl-style-guide/tree/master. I hope that answers your questions, but let me know if anything is not clear.
I've raised PR #1293 to resolve this issue.
Evening @urbite and @JHertz5 ,
Building VSG Knowing that python is interpreted, I was a little puzzled that a 'build' was needed to run vsg. My understanding now (please confirm) is that the build is for creating the wheel file to allow a module to be installed via pip. This could be useful if one wanted to distribute the latest version of vsg to other developers on a team. Then the team members could pip install from the wheel file and not have to clone the vsg repo. Newer versions of the wheel file could be built and distributed as vsg improvements are made.
The build is more of a packaging effort for distribution. Wheel is used to create the package that is eventually pushed to PYPI so it can be pulled down via pip
. I suppose you could use wheel to package a local version of VSG and install it. Maybe that could be a good method to distribute PRs that have not been merged and released yet, or to distribute VSG from HEAD.
project owner (hopefully) accepts PR and merges branch from forked repo
You should check out what is going into the next release . @JHertz5 has the majority of the changes and continues to generate PRs faster than I can merge them.
Thank you again @JHertz5 for your support on this and other issues. I really do appreciate the effort you put into addressing issues other people are having.
The PR looks good, I will merge it to master.
Regards,
--Jeremy
Thanks very much!
Is your feature request related to a problem? Please describe. I'd like a rule to enforce case on the
protected
keyword in a protected type declaration and body. For example, I'd likecorrected to
@JHertz5 I used your issue #1280 as a template. I'll wait for your confirmation that the format of this issue is acceptable before creating additional issues.
I noticed that in issue #1280 the others in the title was highlighted. I was going to do the same for protected using the same markdown as is used in the issue body. That is, ```protected```. But I wasn't sure if it would work and didn't want to have a wonky title with the backticks. How did you get the highlighted
others
in your title.