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

gyp: add missing openssl_fips% to common.gypi #5919

Closed
wants to merge 1 commit into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Mar 27, 2016

Pull Request check-list

Please make sure to review and check all of these items:

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

gyp

Description of change

No comments, it just should be there in some rare cases.

See: atom/node@cba512d

No comments, it just should be there in some rare cases.

See: atom/node@cba512d
@indutny
Copy link
Member Author

indutny commented Mar 27, 2016

cc @zcbenz

@indutny
Copy link
Member Author

indutny commented Mar 27, 2016

cc @nodejs/collaborators

@indutny indutny added the build Issues and PRs related to build files or the CI. label Mar 27, 2016
@jbergstroem
Copy link
Member

@mscdex
Copy link
Contributor

mscdex commented Mar 27, 2016

Perhaps the commit description should target the build subsystem instead of gyp?

@jbergstroem
Copy link
Member

LGTM. Agree with @mscdex, plus perhaps remove the 'no comment' reference since you link to an example of the problem.

@zcbenz
Copy link
Contributor

zcbenz commented Mar 27, 2016

Looks good to me, thanks!

@indutny
Copy link
Member Author

indutny commented Mar 27, 2016

Landed in 26a4a4b, thank you everyone!

@indutny indutny closed this Mar 27, 2016
@indutny indutny deleted the feature/atom-3 branch March 27, 2016 15:35
indutny added a commit that referenced this pull request Mar 27, 2016
See: atom/node@cba512d

PR-URL: #5919
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
evanlucas pushed a commit that referenced this pull request Mar 30, 2016
See: atom/node@cba512d

PR-URL: #5919
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@MylesBorins
Copy link
Contributor

@indutny should this be backported to lts?

@indutny
Copy link
Member Author

indutny commented Mar 30, 2016

I would say go for it! 😉

evanlucas pushed a commit that referenced this pull request Mar 31, 2016
See: atom/node@cba512d

PR-URL: #5919
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this pull request Apr 11, 2016
See: atom/node@cba512d

PR-URL: #5919
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@MylesBorins MylesBorins mentioned this pull request Apr 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants