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

Upgrade formatting, readability, consistency in configure.ac. #5836

Closed
wants to merge 8 commits into from

Conversation

randy-waterhouse
Copy link
Contributor

Helps tidy ./configure --help readability and consistency also. Needs testing on OSX, windows builds.

@jgarzik
Copy link
Contributor

jgarzik commented Feb 27, 2015

ut ACK -- seems like @theuni trivial tree material

@randy-waterhouse
Copy link
Contributor Author

Adding some more as I work through.
@jgarzik should just be trivial if I steer clear of that gray line between formatting and functionality.


# -static is interpreted by libtool, where it has a different meaning.
# In libtool-speak, it's -all-static.
AC_CHECK_LIB([mingwthrd],[main],[], AC_MSG_ERROR([lib missing]))
Copy link
Member

Choose a reason for hiding this comment

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

Although it adds a bit of readability, 'pad with spaces' alignment has the disadvantage that a change on one line makes it necessary to change every line in the table, which makes reviewing changes harder, and can introduce merge conflicts. So I'm generally not in favor of it.

@laanwj
Copy link
Member

laanwj commented Mar 9, 2015

I'll leave this up to @theuni.

I wonder if there is something akin to clang-format for these files. I'm not against increasing readability, but it is very subjective if it is not possible to automate a certain set of rules. At least try to do this all at once, so that we don't break current pulls affecting configure.ac multiple times.

@theuni
Copy link
Member

theuni commented Mar 10, 2015

Sorry for the slow response here, I've been trying to get to the bottom of the AWS block file corruption. I'll review tomorrow.

@theuni
Copy link
Member

theuni commented Mar 12, 2015

@laanwj My thoughts exactly. These are all good cleanups, but without any real formatting rules this is basically a never-ending syntax ping-pong.

@randy-waterhouse I'm not thrilled about all the whitespace/formatting changes because as I see it one of two things will happen:

  1. These will go in, but the next set of changes won't necessarily match.
  2. These go in, then we start policing nits in the future (see header includes as an example there).

My preference would be to take the stuff that is more technically correct (# -> dnl and adding missing [ ] for example), and leave the stuff that's just internal formatting alone.

Re-ordering the options makes sense though, I like it when other programs have theirs organized.

Either way, let's take this over to the trivial tree to discuss, to avoid burning c-i cycles here. Please PR to trivial-next in https://github.com/theuni/bitcoin.

@randy-waterhouse
Copy link
Contributor Author

I agree, it is too trivial to waste our time on a back and forward about formatting style preferences.
You seem to know what you want and you can take from my mods whatever you like.

The file does need cleaning up though it is in a bit of a state.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants