-
Notifications
You must be signed in to change notification settings - Fork 18
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
Check version bump #148
Check version bump #148
Conversation
from .errors import ProtocolError | ||
|
||
def get_changed_feature_files(target_branch): | ||
changed_files = subprocess.getoutput(f"git diff --name-only {target_branch}").split("\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change these to subprocess.run()
which is the "modern" way to do this, operating on a list. So that you don't need to worry about shell escaping. If {file}
(next fn) contains a space, it will break. But if every arg is a separate str in a list passed to subprocess.run() it gets escaped automatically.
value = None, | ||
message = "When changing a feature file, the version number must be bumped" | ||
) | ||
return versions[1] > versions[0] # Check version bump |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah you're using the diff output. I can't think of a way where it would break. I have used https://pypi.org/project/unidiff/ in the past to more properly parse the diff output, but wouldn't necessarily recommend. Also had some issues.
I would add a check though about if len(versions) > 2
. I guess that could happen if you add multiple version tags? Or is that already checked elsewhere. Minor thing.
I wonder if we should check return versions[1] == versions[0] + 1
?
@@ -38,6 +38,16 @@ jobs: | |||
flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics | |||
# exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide | |||
flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics | |||
- name: Setup version bump env vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not necesary. There should be a GITHUB_BASE_REF
environment variable. I would skip most of this. And just check os.environ.get('GITHUB_BASE_REF')
in run()
.
https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables
Minor changes such as fixing typos or re-wording the description do not increment the version It appears that this checking script will need some additional nuance. |
PR is deprecated; all further updates are in #170 |
In case something is modified in a .feature file, the version number needs to be bumped. This scripts checks that automatically. In github CI/CD, it only gets triggered on pull requests and not on push. This will prevent confusing situations, for instance when a .feature file is updated multiple times within a specific branch that is used for a pull request.
Specifically, the number referenced to in the @Version tag. For example in
ifc-gherkin-rules/features/ALA002_Alignment-same-number-of-segments-in-business-logic-and-geometry.feature
Line 2 in 874bb08
Related to #147
As well as related to the issue #97
The purpose is to setup a rule catalog. So that we can output the correct
Then
step of the validation outcome to the end-user without unnecessarily populating the database. However, for this we'll have to ensure we don't output an outdated version of the .feature file.