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

package scripts should use upstream, not origin, to set repository.url from git #363

Closed
rotu opened this issue Sep 14, 2023 · 2 comments · Fixed by #366
Closed

package scripts should use upstream, not origin, to set repository.url from git #363

rotu opened this issue Sep 14, 2023 · 2 comments · Fixed by #366

Comments

@rotu
Copy link
Contributor

rotu commented Sep 14, 2023

The repository URLs are based on assuming that origin is the authoritative url and updates package.json based on this. In the case of this repo, it causes tests to fail locally.

This should be expecting origin to be the default branch to push/pull and upstream to be the authoritative repo because:

Per @lukekarrys in #338 (comment)

@rotu that is due to the autodetection of the origin by git. If you've forked the repo make sure that npm/template-oss is the origin and the remote url for your fork is named something different (eg fork):

❯ git remotes
fork	git@github.com:lukekarrys/template-oss.git (fetch)
fork	git@github.com:lukekarrys/template-oss.git (push)
origin	git@github.com:npm/template-oss.git (fetch)
origin	git@github.com:npm/template-oss.git (push)
@rotu rotu changed the title repository uses origin for setting URLs package scripts should use upstream, not origin, to set repository.url from git Sep 14, 2023
@lukekarrys
Copy link
Contributor

This tool is mainly designed to run in other repos where origin is the default remote.

Probably the best thing to do here is skip that check when specifically in this template-oss repo like we do with the version check.

Alternatively we could do the same thing we do in lib/content/_job.yml and hardcode a repository_owner type variable (which should probably be moved to `lib/content/index.js) and only apply the package.json git url if the current remote matches.

@rotu
Copy link
Contributor Author

rotu commented Sep 16, 2023

This issue does not just impact contributing to the template-oss repo. In npm-cli,

npm run lint

> npm@10.1.0 lint
> eslint "**/*.js"


> npm@10.1.0 postlint
> template-oss-check


Some problems were detected:

-------------------------------------------------------------------

The module file package.json needs to be updated:

  package.json
  ========================================
  "repository.url" is "https://github.com/npm/cli.git", expected "https://github.com/rotu/cli.git"

To correct it: npx template-oss-apply --force

-------------------------------------------------------------------

The module file package.json needs to be updated:

  docs/package.json
  ========================================
  "repository.url" is "https://github.com/npm/cli.git", expected "https://github.com/rotu/cli.git"

To correct it: npx template-oss-apply --force

-------------------------------------------------------------------
...

and it goes on like this for several more packages.

hashtagchris added a commit to npm/stafftools that referenced this issue Jul 23, 2024
<!-- What / Why -->
<!-- Describe the request in detail. What it does and why it's being
changed. -->

By default the gh cli operates against the upstream repo when you run
commands like `gh pr checkout ...`. For the purposes of stafftools, we
want to operate against the forked repo, so we run `gh repo set-default
npm/{name}`.

Tested with `node ./bin/gh.mjs template-oss-fix --install --filter
'name:json-parse-even-better-errors'` after removing
`~/projects/npm/json-parse-even-better-errors`. Also tested with `node
./bin/gh.mjs repos clone --filter "name:agent"`, to ensure clone still
works for non-forked repos.

## References
<!-- Examples:
  Related to #0
  Depends on #0
  Blocked by #0
  Fixes #0
  Closes #0
-->

Change to favor the upstream remote:
npm/template-oss#363

npm/json-parse-even-better-errors#65

cli/cli#9261 (comment)

cli/cli#6777
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 a pull request may close this issue.

2 participants