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

[Meta] Prefered look of Makefiles #9682

Open
ja-pa opened this issue Aug 9, 2019 · 11 comments
Open

[Meta] Prefered look of Makefiles #9682

ja-pa opened this issue Aug 9, 2019 · 11 comments

Comments

@ja-pa
Copy link
Contributor

ja-pa commented Aug 9, 2019

@BKPepe @val-kulkov @neheb
Hi,
I would like to start a discussion about the preferred look of Makefiles.

This is an extension of discussion from PR #9667 (comment)

My idea is, if we could define some guidelines how should Makefile look (variable orders, tabs usage etc.) in a more explicit way. I believe that it can have advantages of in case of change review and possibly in future, this could be used as a specification for a tool which could check that automatically.

@ja-pa
Copy link
Contributor Author

ja-pa commented Aug 9, 2019

I personally prefer template described here by @BKPepe #9399 (comment)

@BKPepe
Copy link
Member

BKPepe commented Aug 9, 2019

I agree we should have a template to which all of we will stick, otherwise, somebody will do it how he/she likes it and we can start with that one, which was mention in that PR. Thanks to @neheb in most Makefiles, there is order in them.

However, it's not updated in OpenWrt documentation either in mwarning's openwrt-examples. We should update both of them.

Another point of this discussion should be about PKG_SOURCE_URL. Do we prefer there .git or not? Talking about this #9626 (comment) . Maybe this point should be discussed with OpenWrt main repository. From my point of view, there should be .git.

I think we are catching mixed indentation and would be good if somewhere can be mention this quote

Jul 28 21:57:57 ynezz: https://www.gnu.org/software/make/manual/html_node/Recipe-Syntax.html - line starts with tab => shell command; spaces => make syntax

On the other hand, we can do it in a community maintained repository how we want to do it, but it should be discussed and updated in OpenWrt main repository as well.

@neheb
Copy link
Contributor

neheb commented Aug 9, 2019

The lack of .git is a GitHub feature. Other git repositories require it.

Maybe download.mk can be modified to pattern match on the URL. If there’s.git at the end, use git by default. Otherwise use curl/wget

@karlp
Copy link
Contributor

karlp commented Aug 9, 2019

I feel this is just an invitation to keep churning things that aren't really relevant 99% of the time.

@ja-pa
Copy link
Contributor Author

ja-pa commented Aug 16, 2019

I think it would be beneficial even just as a recommendation. Right now these pieces of information are written in different issues and MR (like #8738 (comment) ...) and some maintainers insist on them.

@val-kulkov
Copy link
Contributor

@ja-pa : openwrt.org is a Wiki. Go ahead and edit it if you feel the recommendations should be published there.

@BKPepe
Copy link
Member

BKPepe commented Aug 16, 2019

I need to quote #4119 (comment) from @stintel :

We should always aim for consistency, and this commit will hopefully avoid people copying it in new packages, which in turn should result in less changes requested. So less work for both reviewers and contributors.

@karlp
Copy link
Contributor

karlp commented Aug 16, 2019

... and some maintainers insist on them.
And I reject what those maintainers insist on, until they're documented properly.

@hnyman
Copy link
Contributor

hnyman commented Aug 20, 2019

I feel that trying to enforce any mandatory rules for Makefile variable order will not work in real life in the long run. That might lead into abandoned PRs as some authors can't/won't amend their commits, so the PRs would just be there after the first commeting round about Makefile polishing.

Similarly, I do not much see point in the current activity of reordering existing Makefiles. Usually you are just looking at one package at a time, and the exact order of statements does not matter much.

The core thing is to have and maintain packages that do compile and actually work.

@diizzyy
Copy link
Contributor

diizzyy commented Aug 22, 2019

It clearly works for other distros.... (just sayin') however it should be properly documented before being enforced.

Examples:
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/porting-order.html
https://www.freebsd.org/doc/en/books/porters-handbook/porting-portlint.html
https://www.freebsd.org/doc/en/books/porters-handbook/

@champtar
Copy link
Member

In my opinion, the only way to enforce it is via the CI.
When people submit PR, they are ready to make some changes if they have almost instant feedback, but if you wait 2 weeks, this can be discouraging if the only blocker is style.

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

No branches or pull requests

8 participants