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

Hygiene/single code style for all existing macros #403

Merged
merged 3 commits into from Aug 9, 2020
Merged

Hygiene/single code style for all existing macros #403

merged 3 commits into from Aug 9, 2020

Conversation

dr-orlovsky
Copy link
Collaborator

@dr-orlovsky dr-orlovsky commented Jan 25, 2020

While being not really important, had to do hygiene of all internal macros (since many of which I re-use in other places) such they now follow the same standard/coding style for all namespace prefixes (i.e. starting with :: followed by a fully-clarified name).

Also added doc strings to some of the macros

@dr-orlovsky dr-orlovsky changed the title Hygiene for macros Hygiene/single code style for all existing macros Jan 25, 2020
@dr-orlovsky
Copy link
Collaborator Author

FYI, have figured out how the macro system works:

  • use ::std instead of $crate::std: works well both for internal macros and exported. If $crate::std is used, it will not work in exported macros (compiler error)
  • use $crate::core instead of ::core – and you also have to specify pub crate core in lib.rs. ::core is not working with exported macros

@stevenroose
Copy link
Collaborator

Is it worth it to have all this verbosity for macros we don't export?

@dr-orlovsky
Copy link
Collaborator Author

What I am sure is that

  1. it worth to stick to some defined coding style throughout the code and not switch its arbitrary even within a single macro;
  2. be prepared that the code may be forked and maintained by other Bitcoin-related projects outside of the concept of "let's make another wallet [library]" which are not fixed on the idea of limiting their functionality to the set of predefined scripts etc. This means it would be nice to have a library prepared to export macros you don't aim to export now - just not to copy-paste its code into their projects.

@dr-orlovsky
Copy link
Collaborator Author

seems Travis needs reset

@stevenroose
Copy link
Collaborator

stevenroose commented Feb 4, 2020

I restarted the timed out test with cleared caches.

@dr-orlovsky
Copy link
Collaborator Author

I have rebased this onto master. Pls let me know if there any need in this.

@elichai
Copy link
Member

elichai commented Jul 22, 2020

I'm for better macro hygiene, it will also ease our life when we will update to edition 2018.
it also reduces ambiguity if you have the same type defined under different modules

@stevenroose
Copy link
Collaborator

ack 257ca8e

Copy link
Member

@elichai elichai left a comment

Choose a reason for hiding this comment

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

ACK

@elichai elichai merged commit e8bcde4 into rust-bitcoin:master Aug 9, 2020
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.

None yet

3 participants