Skip to content

Commit

Permalink
Introduce --link-module to ./configure
Browse files Browse the repository at this point in the history 
- Allows specifying a _third_party_main outside of the node repository
- Allows embedders to create custom builtin modules outside of node's
  repository

PR-URL: #2497
Reviewed-By: fishrock123 - Jeremiah Senkpiel <[email protected]>
Reviewed-By: jasnell - James M Snell <[email protected]>
Reviewed-By: evanlucas - Evan Lucas <[email protected]>
Reviewed-By: cjihrig - Colin Ihrig <[email protected]>
  • Loading branch information
@bmeck @Fishrock123
bmeck authored and Fishrock123 committed Aug 28, 2015
1 parent c6a54d0 commit cd84f39
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions configure
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ parser.add_option("--fully-static",
help="Generate an executable without external dynamic libraries. This "
"will not work on OSX when using default compilation environment")

parser.add_option("--link-module",
action="append",
dest="linked_module",
help="Path to a JS file to be bundled in the binary as a builtin."
"This module will be referenced by basename without extension."
"Can be used multiple times")

parser.add_option("--openssl-no-asm",
action="store_true",
dest="openssl_no_asm",
Expand Down Expand Up @@ -697,6 +704,9 @@ def configure_node(o):
if options.enable_static:
o['variables']['node_target_type'] = 'static_library'

if options.linked_module:
o['variables']['library_files'] = options.linked_module


def configure_library(lib, output):
shared_lib = 'shared_' + lib
Expand Down

8 comments on commit cd84f39

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

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

@orangemocha ack, this landed without a subsystem, is there any way we can make the jenkins job handle that?

@orangemocha
Copy link
Contributor

Choose a reason for hiding this comment

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

@Fishrock123 : what do you mean by "without a subsystem"?

@orangemocha
Copy link
Contributor

Choose a reason for hiding this comment

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

You mean the stuff like src: in the title? The Jenkins job doesn't touch the commit title. You would like it to? Hard to do that when the PR has multiple commits.

@Trott
Copy link
Member

@Trott Trott commented on cd84f39 Aug 29, 2015

Choose a reason for hiding this comment

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

Don't know much about configuring Jenkins, but maybe the Jenkins job can refuse to start if the first line of the commit message doesn't match a regex like /^[a-z]{3}[,a-z]*: \S/?

Bonus points if it also checks that the second line is empty and the third line is not empty.

Extra magic special bonus points if it checks that the first line is no longer than 50 chars and all the other lines are no longer than 72 chars.

@rvagg
Copy link
Member

@rvagg rvagg commented on cd84f39 Aug 29, 2015

Choose a reason for hiding this comment

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

this is what I've had to go for in changelog-maker to catch all the subsystem/group prefixes that I've come across so far: /^((:?\w|\-|,|, )+):\s*/i

@orangemocha
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds reasonable.

I am hoping that we can somehow inject a script on the page at https://jenkins-iojs.nodesource.com/job/node-accept-pull-request/build?delay=0sec.

If so, the script could go and fetch the PR info from github the moment you enter a PR number, and populate a bunch of text boxes with the commit messages, so you could make edits the commit messages right there!

@silverwind
Copy link
Contributor

Choose a reason for hiding this comment

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

a bunch of text boxes

why not just a single textarea (or even better, something like CodeMirror)?

@orangemocha
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just a single textarea

I meant a bunch of textareas, one per commit message. The script could make as many visible as there are commits in the PR.

Please sign in to comment.