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.

Sign up for GitHub

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

Jump to bottom

build: use do_not_edit variable in configure script #12610

Closed
wants to merge 1 commit into from
Closed

build: use do_not_edit variable in configure script #12610

wants to merge 1 commit into from

Conversation

krydos
Copy link
Contributor

@krydos krydos commented Apr 23, 2017

Hi :)

This is tiny-tiny change just to make our configure script a little bit better.
Since in our configure script we already have do_not_edit variable we can use it in all
places where we actually can. Probably the one place was missed and this PR should fix it.

P.S.
There is no issue for this. Is it ok? Should I create such issue first?

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

build

@krydos
build: use do_not_edit variable where possible
040b1c2 
We should not use hardcoded string to warn users about
file was generated by configure script. Since we already
have do_not_edit variable we can use it
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Apr 23, 2017
addaleax
addaleax approved these changes Apr 23, 2017
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

There is no issue for this. Is it ok?

This is perfectly fine. :)

@gibfahn
Copy link
Member

gibfahn commented Apr 23, 2017 •
edited
Loading

CI: https://ci.nodejs.org/job/node-test-commit/9362/

EDIT: CI is green

@addaleax
Copy link
Member

Thanks, landed in 78e0063!

@addaleax addaleax closed this Apr 27, 2017
addaleax pushed a commit that referenced this pull request Apr 27, 2017
@krydos @addaleax
build: use do_not_edit variable where possible
78e0063 
We should not use hardcoded string to warn users about
file was generated by configure script. Since we already
have do_not_edit variable we can use it

PR-URL: #12610
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@krydos krydos deleted the configure-cleanup branch April 30, 2017 16:15
@evanlucas evanlucas mentioned this pull request May 1, 2017
Merged
evanlucas pushed a commit that referenced this pull request May 1, 2017
@krydos @evanlucas
build: use do_not_edit variable where possible
6836785 
We should not use hardcoded string to warn users about
file was generated by configure script. Since we already
have do_not_edit variable we can use it

PR-URL: #12610
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
evanlucas pushed a commit that referenced this pull request May 2, 2017
@krydos @evanlucas
build: use do_not_edit variable where possible
93b12a5 
We should not use hardcoded string to warn users about
file was generated by configure script. Since we already
have do_not_edit variable we can use it

PR-URL: #12610
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
evanlucas pushed a commit that referenced this pull request May 2, 2017
@krydos @evanlucas
build: use do_not_edit variable where possible
212475b 
We should not use hardcoded string to warn users about
file was generated by configure script. Since we already
have do_not_edit variable we can use it

PR-URL: #12610
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
gibfahn pushed a commit that referenced this pull request May 15, 2017
@krydos @gibfahn
build: use do_not_edit variable where possible
32d7981 
We should not use hardcoded string to warn users about
file was generated by configure script. Since we already
have do_not_edit variable we can use it

PR-URL: #12610
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@krydos @andrew749
build: use do_not_edit variable where possible
fdf58c6 
We should not use hardcoded string to warn users about
file was generated by configure script. Since we already
have do_not_edit variable we can use it

PR-URL: nodejs/node#12610
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibfahn gibfahn gibfahn approved these changes

@richardlau richardlau richardlau approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@krydos @gibfahn @addaleax @jasnell @cjihrig @richardlau @nodejs-github-bot