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

build(bazel): add entry_point_name attr to ng_package #22963

Closed

Conversation

jelbourn
Copy link
Member

@alexeagle not sure if this warrants adding a separate gold setup since I don't think we want to apply this option to the existing example package. I also suspect this attr could be potentially replaced down the road with more intelligence in the rule.

@mary-poppins
Copy link

You can preview a648fcc at https://pr22963-a648fcc.ngbuilds.io/.

# If an explicit entry_point_name is given, use that.
if entry_point_name:
return entry_point_name
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

now that it's an if-else construct, I think it would be better to replace the line below too

if entry_point_name:
  ...
elsif entry_point.find("/") >= 0:
  ...
else:
  return name

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -258,6 +258,9 @@ NG_PACKAGE_ATTRS = dict(NPM_PACKAGE_ATTRS, **dict(ROLLUP_ATTRS, **{
"include_devmode_srcs": attr.bool(default = False),
"readme_md": attr.label(allow_single_file = FileType([".md"])),
"globals": attr.string_dict(default={}),
"entry_point_name": attr.string(
doc = "Name to use when generating bundle files for the primary entry-point.",
Copy link
Contributor

Choose a reason for hiding this comment

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

ooh, we need to do this for all the attributes and then make skydoc do the right thing to generate the API docs

Copy link
Member Author

Choose a reason for hiding this comment

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

Was this what triggered to to go down a skydoc rabbit hole?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so? was a long time ago

@@ -278,11 +281,15 @@ NG_PACKAGE_ATTRS = dict(NPM_PACKAGE_ATTRS, **dict(ROLLUP_ATTRS, **{
# Currently we just borrow the entry point for this, if it looks like
# some/path/to/my/package/index.js
# we assume the files should be named "package.*.js"
def primary_entry_point_name(name, entry_point):
return entry_point.split("/")[-2] if entry_point.find("/") >=0 else name
def primary_entry_point_name(name, entry_point, entry_point_name):
Copy link
Contributor

Choose a reason for hiding this comment

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

this was only a separate function because I needed to call it from somewhere else. now it looks like we should inline this into ng_package_outputs()

Copy link
Member Author

Choose a reason for hiding this comment

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

It's good how it is; smaller, more focused functions are always better.

@jelbourn jelbourn force-pushed the ng_package_entry_point_name branch from a648fcc to 0982876 Compare March 26, 2018 18:16
@mary-poppins
Copy link

You can preview 0982876 at https://pr22963-0982876.ngbuilds.io/.

@jelbourn
Copy link
Member Author

@alexeagle any other comments?

@alexeagle
Copy link
Contributor

nope merge it

@alexeagle alexeagle added the target: major This PR is targeted for the next major release label Mar 26, 2018
@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Mar 26, 2018
@matsko matsko closed this in 2388f24 Mar 27, 2018
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants