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

Update clmtrackr w/ git auto-update #308

Merged
merged 2 commits into from
Jul 7, 2020
Merged

Update clmtrackr w/ git auto-update #308

merged 2 commits into from
Jul 7, 2020

Conversation

klausenbusk
Copy link
Contributor

The auto-update glob is out-of-date, so let's switch to NPM and fix the
glob.

Refs #302

The auto-update glob is out-of-date, so let's switch to NPM and fix the
glob.

Refs #302
@klausenbusk
Copy link
Contributor Author

Hmm, the CI shouldn't fail if the file already exists.

@MattIPv4 MattIPv4 self-requested a review July 4, 2020 20:47
@MattIPv4
Copy link
Member

MattIPv4 commented Jul 4, 2020

Agreed

Copy link
Member

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

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

Thanks :)

packages/c/clmtrackr.json Outdated Show resolved Hide resolved
@klausenbusk
Copy link
Contributor Author

klausenbusk commented Jul 5, 2020 via email

@MattIPv4
Copy link
Member

MattIPv4 commented Jul 5, 2020

We discussed this internally -- unless we need to switch to make the package work, we'd much prefer to stay with the original source that the author chose for auto-updating. We don't have insight into why they chose Git over NPM, but there may be a reason. It also reduces the changes in the diff & reduces the risk of switching to an incorrect package, which has happened in a couple of the other PRs.

@klausenbusk
Copy link
Contributor Author

Fair enough :) Should we update the doc and state that NPM is preferred?

BTW: Do you use a tool to generate the glob url?

Co-authored-by: Matt (IPv4) Cowley <me@mattcowley.co.uk>
@klausenbusk klausenbusk requested a review from MattIPv4 July 5, 2020 20:50
@MattIPv4
Copy link
Member

MattIPv4 commented Jul 7, 2020

Should we update the doc and state that NPM is preferred?

I don't think so, we're happy with either being used. Git cloning is a bit slower for sure, but that's not our biggest worry really. I'm happy for folks to add packages with either as a source.

Do you use a tool to generate the glob url?

I built https://www.digitalocean.com/community/tools/glob a while back for testing globs easily on cdnjs, and even added a feature to quickly pull in files from an NPM package to make the whole process that much faster.

The CI generates glob URLs that go to that tool as well.

@MattIPv4 MattIPv4 changed the title Update clmtrackr w/ npm auto-update Update clmtrackr w/ git auto-update Jul 7, 2020
Copy link
Member

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks!

@MattIPv4 MattIPv4 merged commit 5c7600a into cdnjs:master Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants