-
Notifications
You must be signed in to change notification settings - Fork 37.8k
Bitcoin's cleanner #246
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
Bitcoin's cleanner #246
Conversation
Hm, not sure why we need to change our indentation format. It is one of the most common types of indentation/code format (though possibly some different bracket positioning is more common). Though some of the removal of random spaces at the end of lines and cleaning up number of spaces at various points could be done. |
I thought that no standard has been used regarding the indentation of code, so you're right we don't have to take this commit in account even if I like a bit more this style in my Emacs ;) |
Removing spaces and cleaning up is fine, but the project has well-defined code style rules, which I don't think should be changed without discussion. |
you are absolutely right. |
the indentation and code style was declared and we are all already adapting to it, why change it? i'm also disagreeing with the "whitespace cleanup": while that is an easy trick using the emacs extension, it does provoke unnecessary conflicts when merging master into branches. ultimately i don't see any good reason why to pull this req. |
jaromil, are there any rules regarding the indentation and code style defined by the community of developer around the project ? If yes can any one give me some directives to respect these rules in the future ? Thanks. |
There should be in docs/coding.txt |
Thanks. |
i see one good thing in this operation, but then it should split from all the rest and be proposed as a standalone pull request: in addition to the changes you mentioned, you have also added #ifndef / #define to headers, a common practice to avoid multiple inclusion and redeclarations. as that would be a good step towards getting rid of the headers.h insanity, it'd be good to have it IMHO. but then again, as a separate pull request plz. |
I will separate the change you mentioned from the rest of the pull. If there is anything else to keep, go ahead ;) |
we have a metric ton of pending patches, and unfortunately this pull request is very low priority compared to other changes which change a lot of bitcoin code. whitespace diffs tend to require merging and rebasing downstream for very little value. So this pull request is, and will continue to become, out of date. Other changes should definitely go into separate pull requests. |
Fix and modify alerts system as per DexX's feedback
SigOps are for most blocks less than 5000, with a max of 20000 this is fine. When we go to larger blocks, we don't want to hit that limit and as such it makes sense to adjust the consensus rules to be interpreted as "20k per megabyte". The effective size of the final block is used to decide on the actual amount of sigops allowed, where the size is always counted in whole megabytes, rounded up from the actual byte-count. Important to note that the <= 1MB is still 20k, and as such identical to current consensus rules.
Adapt minting tab colors from Peerunity
4cf815e Add release notes entry for encodings. (Daniel Kraft) 810bc13 Support -name/valueencoding for namecoin-tx. (Daniel Kraft) 20ab39a Handle encodings on the output side. (Daniel Kraft) 482c30e Explicitly use UTF-8 for name_filter regexp check. (Daniel Kraft) ee7fba9 Introduce EncodeNameForMessage for logs / errors. (Daniel Kraft) 80f8ba0 Move names/encoding library to common lib. (Daniel Kraft) 0688f01 Remove ValtypeFromString. (Daniel Kraft) 9367b48 Use configured encoding for "input" names/values. (Daniel Kraft) 7e364eb Add -nameencoding / -valueencoding options. (Daniel Kraft) b6b8483 Add new names/encoding source files. (Daniel Kraft) 0a1c72e Expose univalue's UTF-8 validation logic. (Daniel Kraft) Pull request description: This implements the main part of the proposal in bitcoin#246: New `-nameencoding` and `-valueencoding` options are added, with the possible values of `ascii`, `utf8` and `hex`. They specify in which encoding name and value data is expected in RPC arguments and returned in JSON fields. This fixes a long-standing issue Namecoin had with binary data that was invalid UTF-8 (as well as adding proper support for real binary data in the RPC interface). As a consequence, this likely resolves (or is at least a big part of the solution of) bitcoin#152 and bitcoin#170. Still missing for a full solution of bitcoin#246 is the possibility to specify per-RPC encodings in an `options` RPC argument. For now, the encodings can only be set for the full process at startup time through the config options. Tree-SHA512: b221c3ccf421517be3419aec568e02475c2562fa1e99a70b7c53332ce343dc78a537285183262b270fb676f7c39cf16a9e58cb8160e2e4653211d0daa77e0437
dd92fe3 Allow encoding override in write RPCs. (Daniel Kraft) 3658cca Separate method for decoding value from RPC. (Daniel Kraft) Pull request description: This extends the already existing `options` argument to the "write" RPCs (`name_new`, `name_firstupdate` and `name_update`) to include `nameEncoding` and `valueEncoding`, as proposed in bitcoin#194. Using these options, the encodings can be changed from the configured defaults on a per-RPC basis as needed. This is one of the still missing parts for bitcoin#246, although not the only one (because the RPCs that do not yet have an `options` argument still do not support this). Tree-SHA512: 44f9ba398f7491debcc0f9c33930e6ceca37a793976df7dd6314c626cc45f44da00a73e1c3ddfd26aef2de9db04594ce80b9862562b6b4f3920b6e1f08722f59
…lter obsolete 4a0a288 Remove name_filter. (Daniel Kraft) 695204e Add filtering options for name_scan. (Daniel Kraft) e980bb9 Allow empty string with hex encoding. (Daniel Kraft) 8d3e8cb Support encoding options for name_scan. (Daniel Kraft) e6097b8 Add _encoding / _error fields to help text. (Daniel Kraft) fbbe194 Move help builder for "options" to rpc/names. (Daniel Kraft) f156d78 Add options argument to getNameInfo. (Daniel Kraft) Pull request description: In this change, we add a new optional `options` JSON object argument for `name_scan`. It can be used to override the name and value encodings, implementing bitcoin#246 for `name_scan`. Furthermore, options specific to `name_scan` can be set to filter names based on the number of confirmations, their prefix or a regexp as discussed in bitcoin#237. (`showExpired` is not yet supported, as that will be added in a later step for other RPCs as well, as described in bitcoin#194.) With this extension, `name_scan` now supports everything that `name_filter` does except for the stats mode. Thus, `name_filter` is removed to simplify the RPC interface and reduce the future maintenance burden. Users of `name_filter` should migrate to the new options of `name_scan`. This also finally implements the already closed feature request in bitcoin#16. Tree-SHA512: 1120f0f3ed24dc0c23346c8328cf2f865058118373674a8ee5d8f7233f16c23a3ba4feab5947666acc2ac394b5058b7bae16affb00d1272693ae4cb9abd61ddb
32d8d8a Add options argument for name_pending. (Daniel Kraft) 9cf463b Add options argument to name_list. (Daniel Kraft) 5bb8a2e Add options argument for name_show/history. (Daniel Kraft) Pull request description: This adds an optional `options` argument for the remaining name RPCs: `name_show`, `name_history`, `name_list` and `name_pending`, as suggested in bitcoin#194. For now, the only implemented options are `nameEncoding` and `valueEncoding`, allowing to override the default-configured encodings on a per-RPC basis (implementing another missing part of bitcoin#246). Two name-related RPC methods are remaining without an `options` argument, where I suggest to not add it at all: - `namerawtransaction`: This already has many arguments and adding an `options` argument seems not too useful. `namecoin-tx` can already be used instead, which allows to have arbitrary names and values in hex-encoding. - `sendtoname`: This also already has many arguments so adding `options` would make the signature even more complex. Furthermore, this call is "informally deprecated" anyway, see bitcoin#12, and probably not often used. Tree-SHA512: fe9db50986e05c6adc96ccf4dd4baf904a7d88cc19ff043f01613da7da3da818accb7c73083489ff87e6263dee7e615628b3d27081a433b526ac317b4d1f4f72
Hi guys, first let's say that I really appreciate the job you're doing. So thanks to all of you. And kindly consider the little updates that I am requesting. It's just a whitespace cleaning and some indentations added in the code. Regards,