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.

Sign up for GitHub

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

Jump to bottom

Retrieve version from version.txt #2434

Closed
wants to merge 5 commits into from
Closed

Retrieve version from version.txt #2434

wants to merge 5 commits into from

Conversation

atalman
Copy link
Contributor

@atalman atalman commented Jun 2, 2022 •
edited
Loading

Retrieve version from version.txt
These improvement introduced in following PR: pytorch/vision#6117
In addition to this we add version.txt file to help us manage torchaudio version

@atalman atalman requested review from mthrok and nateanl June 2, 2022 15:58
mthrok
mthrok reviewed Jun 2, 2022
View reviewed changes
Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

Could we split the PR in two?. One for changes related to version.txt and the other for other M1 changes.

packaging/pkg_helpers.bash Outdated
# command below yields 0.10.0 from version.txt containing 0.10.0a0
_VERSION_BASE=$( cut -f 1 -d a "$SCRIPT_DIR/../version.txt" )
else
_VERSION_BASE="$1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there an existing use case for this else branch?
If not, it might introduce accidental/unexpected issue.
I think validating $# -eq 0 is more error prof.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, true we want to keep things simple and retrieve either from $BUILD_VERSION or version.txt . Made changes accordingly

@atalman
Copy link
Contributor Author

atalman commented Jun 2, 2022 •
edited
Loading

Could we split the PR in two?. One for changes related to version.txt and the other for other M1 changes.

Hm, I think in original M1 PR, the version was incorrect. But sure I can get the M1 changes out of this and then post another PR for M1. Its true splitting the PR seems cleaner

@atalman atalman changed the title Retrieve version from version.txt and small m1 improvements Retrieve version from version.txt Jun 2, 2022
@atalman atalman requested a review from mthrok June 2, 2022 16:44
@facebook-github-bot
Copy link
Contributor

@atalman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@github-actions
Copy link

github-actions bot commented Jun 2, 2022

Hey @atalman.
You merged this PR, but labels were not properly added. Please add a primary and secondary label (See https://github.com/pytorch/audio/blob/main/.github/process_commit.py)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mthrok mthrok mthrok approved these changes

@nateanl nateanl Awaiting requested review from nateanl

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@atalman @facebook-github-bot @mthrok