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

refactor: move VERSION to src/folder #1148 #1149

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

CNLHC
Copy link
Contributor

@CNLHC CNLHC commented Dec 1, 2022

This fixes #1148.

@CNLHC
Copy link
Contributor Author

CNLHC commented Dec 1, 2022

x.py is only invoked from the project root so using relative dir in x.py seems to be ok.

@CNLHC CNLHC marked this pull request as ready for review December 1, 2022 08:52
@git-hulk git-hulk requested a review from tisonkun December 1, 2022 09:19
@tisonkun tisonkun requested a review from PragmaTwice December 1, 2022 09:22
@PragmaTwice PragmaTwice mentioned this pull request Dec 1, 2022
2 tasks
@tisonkun
Copy link
Member

tisonkun commented Dec 1, 2022

I'd prefer to write VERSION.txt instead of .VERSION. There's no reason for the VERSION file to be hidden.

@PragmaTwice for #1148 (comment), I'd say that version.h.in is not a typical source file also, but we host all source files and meta source files (that generates source files under src). You can think of proto definitions.

@CNLHC
Copy link
Contributor Author

CNLHC commented Dec 1, 2022

I'd prefer to write VERSION.txt instead of .VERSION. There's no reason for the VERSION file to be hidden.

@PragmaTwice for #1148 (comment), I'd say that version.h.in is not a typical source file also, but we host all source files and meta source files (that generates source files under src). You can think of proto definitions.

I agree with @tisonkun that the VERSION should be placed in the src because this file does influence the output binary and can be regarded as some kind of source file.

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

@PragmaTwice Another approach is that we thoroughly remove the VERSION file but pass the version by CMake option and default to 999.999.999 :)

@PragmaTwice
Copy link
Member

PragmaTwice commented Dec 1, 2022

@PragmaTwice Another approach is that we thoroughly remove the VERSION file but pass the version by CMake option and default to 999.999.999 :)

It seems hard to do so in a source distribution, while users are required to specify a version number to build.

@tisonkun
Copy link
Member

tisonkun commented Dec 1, 2022

It seems hard to do so in a source distribution, while users are required to specify a version number to build.

Yep..

@tisonkun
Copy link
Member

tisonkun commented Dec 2, 2022

Thanks for your contribution! Merging...

@tisonkun tisonkun merged commit a8bf8cd into apache:unstable Dec 2, 2022
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.

Move VERSION file to src folder
3 participants