Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

✨ Add support for --registry with audit command #7263

Closed
wants to merge 4 commits into from

Conversation

charlyx
Copy link

@charlyx charlyx commented May 11, 2019

Summary

Hi 👋

First of all, thanks for your work!
I use yarn everyday and I love its workspaces feature 😄

This PR adds support for the --registry flag when running audit command as described in #7012 .

Test plan

I have added a test case where I check if the audit command is calling the registry specified with the high flag --registry.

@buildsize
Copy link

buildsize bot commented May 11, 2019

File name Previous Size New Size Change
yarn-[version].noarch.rpm 1.11 MB 1.11 MB -47 bytes (0%)
yarn-[version].js 4.48 MB 4.48 MB 46 bytes (0%)
yarn-legacy-[version].js 4.67 MB 4.67 MB 45 bytes (0%)
yarn-v[version].tar.gz 1.12 MB 1.12 MB 2.24 KB (0%)
yarn_[version]all.deb 816.96 KB 817.09 KB 142 bytes (0%)

@charlyx charlyx marked this pull request as ready for review May 12, 2019 08:15
@PtrTn
Copy link

PtrTn commented May 15, 2019

Thanks for creating this PR. Personally I would love to have this feature merged ❤️

@wesselvanderlinden
Copy link

I would really like this PR to be merged too :D

@@ -145,6 +146,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
const audit = new Audit(config, reporter, {
groups: flags.groups || OWNED_DEPENDENCY_TYPES,
level: flags.level || DEFAULT_LOG_LEVEL,
registry: flags.registry,

Choose a reason for hiding this comment

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

Why don't you add || YARN_REGISTRY here instead of in the _fetchAudit method?
Here it'll look more consistent, in my opinion.

@dobriai
Copy link

dobriai commented Feb 18, 2020

This explicit command line option is fine, but why isn't the registry specified in .yarnrc used for audit in the first place?
Yarn appears to use a custom registry for pulling down packages, but insists on its own, hard-coded URL for audit - why?

@@ -238,7 +240,7 @@ export default class Audit {

async _fetchAudit(auditTree: AuditTree): Object {
let responseJson;
const registry = YARN_REGISTRY;
const registry = this.options.registry || YARN_REGISTRY;
Copy link

Choose a reason for hiding this comment

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

Even better, maybe add something like || String(this.config.getOption('registry')).replace(/\/+$/, '') in the middle, so we also also consult the config file(s)? (Credit to PR #6484 for that!)

@gbhasha
Copy link

gbhasha commented Apr 27, 2020

When this PR will be merged?
It would be great, it also uses the registry specified in the .yarnrc file for audit.

@schdief
Copy link

schdief commented Apr 30, 2020

would love to see this merged, thx for your effort guys!

@BLoeT
Copy link

BLoeT commented May 19, 2020

Hey Guys,

Unfortunately we need this feature for our project.

Could you pls merge this, if everything is good?

@nemoDreamer
Copy link

Maybe this PR needs a push so that the Yarn Acceptance Tests pass? Seems like yarn team might not look at it until it does? 😭

@funilrys
Copy link

funilrys commented Jun 24, 2021

I had a little giggle about this. I looked at the source code before the #7012 issue.

The patch looks good - in my opinion. @charlyx can you (or someone with high privileges) close and reopen this PR. It will reinforce the run of the checks.

Cheers!

@schmidlidev
Copy link

Any update on this?

@charlyx charlyx closed this Jul 12, 2021
@nemoDreamer
Copy link

Uhm, @charlyx, the original request from @funilrys was if you could close and RE-OPEN this PR, to kick the build checks?

@struys
Copy link

struys commented Mar 3, 2022

@funilrys @charlyx mind taking another look at this?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.