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

Remove handwritten makefiles to keep only one build system #576

Merged
merged 3 commits into from
May 13, 2022

Conversation

PragmaTwice
Copy link
Member

@PragmaTwice PragmaTwice commented May 12, 2022

This resolves #574.

@tisonkun
Copy link
Member

@PragmaTwice we may still keep some targets in Makefile such as lints to running checker.

In a separated pass we can consider whether adopt Makefile to do such job or port them in scripts or other approaches.

NOTICE: CI failed because of make lint missing a Makefile.

@PragmaTwice
Copy link
Member Author

PragmaTwice commented May 13, 2022

@PragmaTwice we may still keep some targets in Makefile such as lints to running checker.

In a separated pass we can consider whether adopt Makefile to do such job or port them in scripts or other approaches.

NOTICE: CI failed because of make lint missing a Makefile.

I think lint is the only target which is out of the responsibility of cmake (not sure, becuase actually it can be written into cmake), and it is quite simple, only call two scripts, cppcheck.sh and cpplint.sh.

Hence I consider to remove the target to delete all Makefiles to make sure these handwritten makefiles will not conflict with cmake-generated makefiles (i.e. call cmake . directly on the root directory of the project, which will overwrite current makefiles).

@tisonkun
Copy link
Member

@PragmaTwice thanks for your investigation! SGTM do the replacement since it's trivial.

@PragmaTwice PragmaTwice marked this pull request as ready for review May 13, 2022 08:14
Copy link
Member

@git-hulk git-hulk left a comment

Choose a reason for hiding this comment

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

LGTM, I like this PR.

@tisonkun tisonkun requested a review from ShooterIT May 13, 2022 10:04
Copy link
Member

@ShooterIT ShooterIT left a comment

Choose a reason for hiding this comment

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

Cool job, it is much clear, thanks!

There are many bug reports about linking snappy when using make, i think this PR can also resolve this problem.

@tisonkun
Copy link
Member

Merging...

@tisonkun tisonkun merged commit 518549f into apache:unstable May 13, 2022
@tisonkun
Copy link
Member

@PragmaTwice thanks for your contribution!

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.

Proposal: Remove handwritten makefiles to keep only one build system
4 participants