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

Show available dependencies in the UI #500

Merged
merged 2 commits into from
Jun 5, 2017
Merged

Show available dependencies in the UI #500

merged 2 commits into from
Jun 5, 2017

Conversation

matteofigus
Copy link
Member

Fixes #490

Each dependency will be linked to either the package homepage or the node docs page (in case of core dependency).

screen shot 2017-06-02 at 23 49 22

Copy link
Member

@mattiaerre mattiaerre left a comment

Choose a reason for hiding this comment

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

over to you @matteofigus I think this is a very nice step; I am wondering if we can expose this info as an API as well; something like: GET /api/v1/dependencies

p.release #{dependency.name} (node.js core dependency)
else
p.release #{dependency.name}@#{dependency.version}

Copy link
Member

Choose a reason for hiding this comment

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

add newline please

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch @mattiaerre , what you think of adding prettier in the near future so that we don't have much nits anymore

Copy link
Member

Choose a reason for hiding this comment

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

I'd love to have that @nickbalestra 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

ok done

each dependency in availableDependencies
.componentRow.row.table
a(href=dependency.link, target="_blank")
if dependency.core
Copy link
Member

Choose a reason for hiding this comment

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

do you think we should list core dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

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

Core deps are subject to be whitelisted too, so I think it's useful to show them. You can see 'url' in the screenshot example

Copy link
Member

Choose a reason for hiding this comment

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

ok, I see. I thought they were implicitly available for us 😊

@mattiaerre
Copy link
Member

mattiaerre commented Jun 4, 2017

Happy to approve if you answer my main question here:

#500 (review)

thanks!

// cc @matteofigus @nickbalestra

Copy link
Contributor

@nickbalestra nickbalestra left a comment

Choose a reason for hiding this comment

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

Great Job!

@matteofigus
Copy link
Member Author

matteofigus commented Jun 5, 2017

@mattiaerre I would be ok on having an api endpoint for that, but I would leave it to a separate PR.

Perhaps we can open an issue first to discuss, as I have opinions to discuss in regards of versioning, the url structure, authentication, and the endpoint scope (one for deps? or one generic with all the details)?

@mattiaerre
Copy link
Member

@matteofigus very good then, as @corlobepy and I would like to work on a list of plugins as well and I was more inclined to expose them via an API so I was thinking that maybe we can have an API endpoint for deps as well. are you happy if we create a couple of issues and PRs on this particular subject? thanks

@mattiaerre mattiaerre merged commit 23c8eb7 into master Jun 5, 2017
@mattiaerre mattiaerre deleted the deps-ui branch June 5, 2017 17:46
@matteofigus
Copy link
Member Author

Sure let's create an issue and discuss there ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants