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

src: make base64.h self-contained #17177

Closed
wants to merge 1 commit into
base: master
from

Conversation

@danbev
Member

danbev commented Nov 21, 2017

This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.
@gireeshpunathil

This comment has been minimized.

Show comment
Hide comment
@gireeshpunathil

gireeshpunathil Nov 21, 2017

Member

I am sorry, but unable to follow - by including a new header (that carry some types required in the file which were resolved otherwise through different route) what benefit we foresee to the compilation unit?

Member

gireeshpunathil commented Nov 21, 2017

I am sorry, but unable to follow - by including a new header (that carry some types required in the file which were resolved otherwise through different route) what benefit we foresee to the compilation unit?

@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev Nov 21, 2017

Member

I am sorry, but unable to follow - by including a new header (that carry some types required in the file which were resolved otherwise through different route) what benefit we foresee to the compilation unit?

One benefit is that making changes to util.h will not affect base64.h, so the types that are used by this header are also included by it. Google's C++ Style Guide has some more details plus I recall this being brought up on previous pull requests I've made where I missed this.

Member

danbev commented Nov 21, 2017

I am sorry, but unable to follow - by including a new header (that carry some types required in the file which were resolved otherwise through different route) what benefit we foresee to the compilation unit?

One benefit is that making changes to util.h will not affect base64.h, so the types that are used by this header are also included by it. Google's C++ Style Guide has some more details plus I recall this being brought up on previous pull requests I've made where I missed this.

@aqrln

aqrln approved these changes Nov 21, 2017

@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev Nov 27, 2017

Member

Landed in 4ca4db0

Member

danbev commented Nov 27, 2017

Landed in 4ca4db0

@danbev danbev closed this Nov 27, 2017

@danbev danbev deleted the danbev:base64-include-stdint branch Nov 27, 2017

danbev added a commit that referenced this pull request Nov 27, 2017

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: #17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

MylesBorins added a commit that referenced this pull request Dec 12, 2017

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: #17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

MylesBorins added a commit that referenced this pull request Dec 12, 2017

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: #17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

MylesBorins added a commit that referenced this pull request Dec 12, 2017

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: #17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

@MylesBorins MylesBorins referenced this pull request Dec 12, 2017

Merged

v9.3.0 proposal #17631

gibfahn added a commit that referenced this pull request Dec 19, 2017

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: #17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

gibfahn added a commit that referenced this pull request Dec 19, 2017

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: #17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

@gibfahn gibfahn referenced this pull request Dec 20, 2017

Closed

v8.9.4 proposal #17772

gibfahn added a commit that referenced this pull request Dec 20, 2017

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: #17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

@gibfahn gibfahn referenced this pull request Dec 20, 2017

Merged

v8.9.4 proposal #17774

@MylesBorins MylesBorins referenced this pull request Dec 20, 2017

Merged

v6.12.3 proposal #17776

msoechting added a commit to hpicgs/node that referenced this pull request Feb 5, 2018

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: nodejs#17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

src: make base64.h self-contained
This commit includes stdint.h (for uint8_t, uint32_t, and int8_t) to
make it self-contained.

PR-URL: nodejs#17177
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment