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

feat: add elixir support #1739

Merged
merged 1 commit into from
Mar 31, 2021
Merged

feat: add elixir support #1739

merged 1 commit into from
Mar 31, 2021

Conversation

admons
Copy link
Contributor

@admons admons commented Mar 18, 2021

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Adds support for elixir ecosystem

@admons admons requested review from a team as code owners March 18, 2021 00:19
@admons admons marked this pull request as draft March 18, 2021 00:19
lili2311
lili2311 reviewed Mar 18, 2021
View reviewed changes
src/lib/plugins/index.ts Outdated
@@ -55,6 +56,9 @@ export function loadPlugin(
case 'cocoapods': {
return cocoapodsPlugin;
}
case 'elixir': {
Copy link
Contributor

@lili2311 lili2311 Mar 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

would a new language not be added via the new ecosystems flow now? I believe @darscan mentioned all new languages/plugins now go via that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, this should be using the new flow. @admons let's chat

@admons admons force-pushed the feat/add-elixir-support branch 2 times, most recently from 90a4c3a to a0c4894 Compare March 24, 2021 15:46
@admons admons force-pushed the feat/add-elixir-support branch from a0c4894 to bf35fec Compare March 29, 2021 16:06
@admons admons marked this pull request as ready for review March 29, 2021 16:58
dtrunley-snyk
dtrunley-snyk reviewed Mar 29, 2021
View reviewed changes
src/lib/detect.ts Outdated
@@ -31,6 +31,7 @@ const DETECTABLE_FILES: string[] = [
'Podfile.lock',
'pyproject.toml',
'poetry.lock',
// 'mix.exs', // todo: remove when hex is going GA
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of interest do we not need mix.lock here too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Of course, was just interested if it was a new approach or not 👍

dtrunley-snyk
dtrunley-snyk reviewed Mar 29, 2021
View reviewed changes
src/lib/package-managers.ts Outdated
@@ -49,6 +51,7 @@ export const GRAPH_SUPPORTED_PACKAGE_MANAGERS: SupportedPackageManagers[] = [
'yarn',
'rubygems',
'poetry',
'hex',
Copy link
Contributor

@dtrunley-snyk dtrunley-snyk Mar 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Had a chat with @anthogez as I wanted to flag that this might be required for Gradle (I had incorrectly recalled something about Gradle moving to dep-graphs but it was actually around introducing support cycles).
It could definitely be better named but we don't need to add hex here if we build the dep graph from scratch, like Go (so it sounds like Poetry also shouldn't be there). This variable is for package managers that build a dep tree and convert to dep graph.

@dtrunley-snyk
Copy link
Contributor

We should add a test in here:
https://github.com/snyk/snyk/tree/master/test/acceptance/cli-test

@admons admons force-pushed the feat/add-elixir-support branch 3 times, most recently from ca5a5d5 to f3c8e87 Compare March 29, 2021 18:45
anthogez
anthogez reviewed Mar 29, 2021
View reviewed changes
package.json Outdated
@@ -78,6 +78,7 @@
"@snyk/graphlib": "^2.1.9-patch.3",
"@snyk/inquirer": "^7.3.3-patch",
"@snyk/snyk-cocoapods-plugin": "2.5.2",
"@snyk/snyk-hex-plugin": "^1.0.0",
Copy link
Contributor

@anthogez anthogez Mar 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Please remove ^ from "@snyk/snyk-hex-plugin": "^1.0.0"
It could bring involuntary breaking changes in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I'm used to projects with lock file

@admons admons force-pushed the feat/add-elixir-support branch from f3c8e87 to 9572d2e Compare March 29, 2021 20:11
@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2021 •
edited by admons
Loading

Expected release notes (by @admons)

features:
add elixir support (9572d2e)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

@admons admons merged commit 5447a41 into master Mar 31, 2021
@admons admons deleted the feat/add-elixir-support branch March 31, 2021 15:35
@snyk-bot snyk-bot mentioned this pull request Jun 3, 2021
Open
@cloudgeek7 cloudgeek7 mentioned this pull request Jun 19, 2022
Open
@aliscco aliscco mentioned this pull request Jun 19, 2022
Open
@cloudgeek7 cloudgeek7 mentioned this pull request Jun 20, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Jun 20, 2022
Open
@dicosnyk dicosnyk mentioned this pull request Jul 15, 2022
Closed
@snyk-bot snyk-bot mentioned this pull request Sep 30, 2022
Closed
@shaniHerz shaniHerz mentioned this pull request Oct 4, 2022
Closed
@nathan-snyk nathan-snyk mentioned this pull request Oct 5, 2022
Open
@nathan-snyk nathan-snyk mentioned this pull request Jan 10, 2023
Open
@snyk-bot snyk-bot mentioned this pull request Apr 27, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darscan darscan darscan left review comments

@lili2311 lili2311 lili2311 left review comments

@anthogez anthogez anthogez left review comments

@dtrunley-snyk dtrunley-snyk dtrunley-snyk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@admons @dtrunley-snyk @darscan @lili2311 @anthogez