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

Version number pulled from version.in file to constant.py #662

Merged
merged 8 commits into from
Apr 13, 2020

Conversation

srbhss
Copy link
Contributor

@srbhss srbhss commented Mar 19, 2020

Issue #641
Automatically pull the version number from VERSION.in file.
Default value is set to 'UNKNOWN'.

issue AcademySoftwareFoundation#641 
Using os module pulled the version number from VERSION.in file.
Default value is set to 'UNKNOWN'.
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 19, 2020

CLA Check
The committers are authorized under a signed CLA.

@bcipriano
Copy link
Collaborator

@srbhss Hi, I appreciate the work but please don't work on issues or send PRs for issues that you are not assigned to.

This is laid out in our Contributing guide: https://www.opencue.io/contributing/opencue/contributing/#github-issues

@srbhss
Copy link
Contributor Author

srbhss commented Mar 20, 2020

@bcipriano Sir please assign me to this issue , I would like to work on this issue.

cuegui/cuegui/Constants.py Outdated Show resolved Hide resolved
cuegui/cuegui/Constants.py Outdated Show resolved Hide resolved
@srbhss
Copy link
Contributor Author

srbhss commented Mar 25, 2020

@bcipriano please give it a look.

Copy link
Collaborator

@bcipriano bcipriano left a comment

Choose a reason for hiding this comment

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

It's close, just a final change requested to set the default value in the event the file doesn't exist.

cuegui/cuegui/Constants.py Outdated Show resolved Hide resolved
cuegui/cuegui/Constants.py Outdated Show resolved Hide resolved
@srbhss
Copy link
Contributor Author

srbhss commented Mar 26, 2020

@bcipriano Sir I have added "1.3.0" as the default version if file doesn't exist as suggested.
Ready for final reviews and check.

Copy link
Collaborator

@bcipriano bcipriano left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks!

@srbhss
Copy link
Contributor Author

srbhss commented Mar 28, 2020

@gregdenton @larsbijl Please review this PR.

@larsbijl
Copy link
Contributor

@smith1511 would this logic of ../../.. have issue on Windows?

@bcipriano
Copy link
Collaborator

I've just tested it out on Windows and it works as expected

Copy link
Collaborator

@gregdenton gregdenton left a comment

Choose a reason for hiding this comment

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

LGTM.

@bcipriano bcipriano merged commit 38e3e05 into AcademySoftwareFoundation:master Apr 13, 2020
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.

None yet

4 participants