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

feat: add branch and pkg name to monitor analytics #543

Merged
merged 1 commit into from
Jun 2, 2019

Conversation

odinn1984
Copy link
Contributor

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

What does this PR do?

We want to add branch and project name to the data that we send to analytics.

@odinn1984 odinn1984 self-assigned this May 29, 2019
analytics.add('branch', target.branch);
}

analytics.add('projectName', pkg.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

project name will be calculated in Registry in the end and will unlikely be this name. Another important property is --project-name option as it is the project name override that is also used to calculate project name on the other end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @lili2311 that's good to know. so that's why we're not sending it from CLI i guess... ok I'll talk to the BI team about it since it was their request and see if pkg name is enough for them and why they need that. pkg.name will always b there though no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok @lili2311 looks like we can send the ID as the project name from my few tests... wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

which ID? Cli never knows if there is an existing project or not until we monitor. So when monitor returns an id thats the only time we have something solid. Will that be enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the meta.id one... looks like it's similar to the project name? or is it just in npm and maven?

@odinn1984 odinn1984 force-pushed the feat/add_monitor_branch_pname_analytics branch from 188ebd8 to ece0759 Compare May 30, 2019 08:35
@odinn1984 odinn1984 force-pushed the feat/add_monitor_branch_pname_analytics branch 3 times, most recently from 55650d4 to cb883ea Compare June 2, 2019 09:55
@odinn1984 odinn1984 force-pushed the feat/add_monitor_branch_pname_analytics branch from cb883ea to b91d2ae Compare June 2, 2019 10:05
@odinn1984 odinn1984 merged commit 305ac09 into master Jun 2, 2019
@odinn1984 odinn1984 deleted the feat/add_monitor_branch_pname_analytics branch June 2, 2019 12:03
@snyksec
Copy link

snyksec commented Jun 2, 2019

🎉 This PR is included in version 1.171.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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