Repository navigation
fix: git pull && npm i should not change things - #126
Merged
Merged
Conversation
Pulling latest `main` and running `npm i` currently changes `package-lock.json`. Specifically, it adds the local packages from the `packages` folder. Let's check in this change.
pselle
approved these changes
Nov 14, 2025
pselle
left a comment
pselle
left a comment
Contributor
There was a problem hiding this comment.
I think the dependabot PRs that update dependencies rip these out. There might be some knob turning for a dependabot.yml that could build the clients and make sure these don't get removed.
chadoh
added a commit
that referenced
this pull request
Nov 14, 2025
IS THIS A GOOD IDEA?? Force check-in everything in `packages` and `src/contracts`! Why? We recently ([1]) force-checked-in a `src/contracts/guess_the_number.ts` stub so that linting would work on the main branch. The thinking being: 👉 People who read instructions (all 50% of them) will `scaffold init` the project, which will not have _any_ git, and so when they first `git init` and `git add`, the gitignore rules will be respected and the placeholder `src/contracts/*` file(s) will not be checked in. In this PR, I went and used more features of the full, non-stubbed version of `guess_the_number`, which means that the repo, having only the stub, started failing linting again, because it doesn't know about things like `tx.signAndSend`. Rather than updating the stub, I wonder if it makes more sense, under the same logic as above, to force-check-in _all_ these packages and artifacts. Pros ---- - Eslint on GH CI has full context, and won't say my code fails. - It's also future-proof. As we add more features showing how to use different aspects of these generated libraries, we won't need to keep playing whack-a-mole with our stub. - Dependabot! Dependabot has been stripping the stuff in `packages` out of our `package-lock.json`, which we then need to re-add (see #126). If the stuff in `packages` is force-checked-in, we should work around this problem. - Less-treacherous contributing workflow: with the stub, anytime you pulled `stellar-scaffold-frontend` and ran `run dev`, you ended up overwriting the stub. This then led to ugly/confusing git changes that you needed to ignore. We should have _fewer_ such changes now, since the force-checked-in files _should_ match what's generated. Most of the time. When the contracts change, the generated packages will change too, and we will then end up with git diffs. But we actually WANT to check these in, to keep the described behavior here consistent. Cons ---- - In order to achieve all the Pros listed above, we should _not_ include a comment saying that these files are stubs. We should just leave them as-is, to avoid git diffs as much as possible. Given that the standard `scaffold init` workflow should result in these files *NOT* being included, though, this seems fine? And for people ignoring instructions and cloning `scaffold-stellar-frontend` directly, well, they'll have functioning packages and `src/contracts` imports. At least to start off. And then they'll change things and end up with confusing git diffs and seek support at that point. [1]: #121 (comment)
chadoh
added a commit
that referenced
this pull request
Nov 14, 2025
IS THIS A GOOD IDEA?? Force check-in everything in `packages` and `src/contracts`! Why? We recently ([1]) force-checked-in a `src/contracts/guess_the_number.ts` stub so that linting would work on the main branch. The thinking being: 👉 People who read instructions (all 50% of them) will `scaffold init` the project, which will not have _any_ git, and so when they first `git init` and `git add`, the gitignore rules will be respected and the placeholder `src/contracts/*` file(s) will not be checked in. In this PR, I went and used more features of the full, non-stubbed version of `guess_the_number`, which means that the repo, having only the stub, started failing linting again, because it doesn't know about things like `tx.signAndSend`. Rather than updating the stub, I wonder if it makes more sense, under the same logic as above, to force-check-in _all_ these packages and artifacts. Pros ---- - Eslint on GH CI has full context, and won't say my code fails. - It's also future-proof. As we add more features showing how to use different aspects of these generated libraries, we won't need to keep playing whack-a-mole with our stub. - Dependabot! Dependabot has been stripping the stuff in `packages` out of our `package-lock.json`, which we then need to re-add (see #126). If the stuff in `packages` is force-checked-in, we should work around this problem. - Less-treacherous contributing workflow: with the stub, anytime you pulled `stellar-scaffold-frontend` and ran `run dev`, you ended up overwriting the stub. This then led to ugly/confusing git changes that you needed to ignore. We should have _fewer_ such changes now, since the force-checked-in files _should_ match what's generated. Most of the time. When the contracts change, the generated packages will change too, and we will then end up with git diffs. But we actually WANT to check these in, to keep the described behavior here consistent. Cons ---- - In order to achieve all the Pros listed above, we should _not_ include a comment saying that these files are stubs. We should just leave them as-is, to avoid git diffs as much as possible. Given that the standard `scaffold init` workflow should result in these files *NOT* being included, though, this seems fine? And for people ignoring instructions and cloning `scaffold-stellar-frontend` directly, well, they'll have functioning packages and `src/contracts` imports. At least to start off. And then they'll change things and end up with confusing git diffs and seek support at that point. I feel like maybe the pros outweigh the cons? Wdyt? What else is in this commit =========================== - Tell eslint to ignore the temporary files in `target/packages`, since they were causing tremendous noise when linting locally - Suppress git diffs for `packages/*` and `src/contracts/*`, since these are now checked-in artifacts and don't need to clutter git diffs. [1]: #121 (comment)
chadoh
added a commit
that referenced
this pull request
Nov 14, 2025
IS THIS A GOOD IDEA?? Force check-in everything in `packages` and `src/contracts`! Why? We recently ([1]) force-checked-in a `src/contracts/guess_the_number.ts` stub so that linting would work on the main branch. The thinking being: 👉 People who read instructions (all 50% of them) will `scaffold init` the project, which will not have _any_ git, and so when they first `git init` and `git add`, the gitignore rules will be respected and the placeholder `src/contracts/*` file(s) will not be checked in. In this PR, I went and used more features of the full, non-stubbed version of `guess_the_number`, which means that the repo, having only the stub, started failing linting again, because it doesn't know about things like `tx.signAndSend`. Rather than updating the stub, I wonder if it makes more sense, under the same logic as above, to force-check-in _all_ these packages and artifacts. Pros ---- - Eslint on GH CI has full context, and won't say my code fails. - It's also future-proof. As we add more features showing how to use different aspects of these generated libraries, we won't need to keep playing whack-a-mole with our stub. - Dependabot! Dependabot has been stripping the stuff in `packages` out of our `package-lock.json`, which we then need to re-add (see #126). If the stuff in `packages` is force-checked-in, we should work around this problem. - Less-treacherous contributing workflow: with the stub, anytime you pulled `stellar-scaffold-frontend` and ran `run dev`, you ended up overwriting the stub. This then led to ugly/confusing git changes that you needed to ignore. We should have _fewer_ such changes now, since the force-checked-in files _should_ match what's generated. Most of the time. When the contracts change, the generated packages will change too, and we will then end up with git diffs. But we actually WANT to check these in, to keep the described behavior here consistent. Cons ---- - In order to achieve all the Pros listed above, we should _not_ include a comment saying that these files are stubs. We should just leave them as-is, to avoid git diffs as much as possible. Given that the standard `scaffold init` workflow should result in these files *NOT* being included, though, this seems fine? And for people ignoring instructions and cloning `scaffold-stellar-frontend` directly, well, they'll have functioning packages and `src/contracts` imports. At least to start off. And then they'll change things and end up with confusing git diffs and seek support at that point. I feel like maybe the pros outweigh the cons? Wdyt? What else is in this commit =========================== - Tell eslint to ignore the temporary files in `target/packages`, since they were causing tremendous noise when linting locally - Suppress git diffs for `packages/*` and `src/contracts/*`, since these are now checked-in artifacts and don't need to clutter git diffs. [1]: #121 (comment)
chadoh
added a commit
that referenced
this pull request
Nov 14, 2025
IS THIS A GOOD IDEA?? Force check-in everything in `packages` and `src/contracts`! Why? We recently ([1]) force-checked-in a `src/contracts/guess_the_number.ts` stub so that linting would work on the main branch. The thinking being: 👉 People who read instructions (all 50% of them) will `scaffold init` the project, which will not have _any_ git, and so when they first `git init` and `git add`, the gitignore rules will be respected and the placeholder `src/contracts/*` file(s) will not be checked in. In this PR, I went and used more features of the full, non-stubbed version of `guess_the_number`, which means that the repo, having only the stub, started failing linting again, because it doesn't know about things like `tx.signAndSend`. Rather than updating the stub, I wonder if it makes more sense, under the same logic as above, to force-check-in _all_ these packages and artifacts. Pros ---- - Eslint on GH CI has full context, and won't say my code fails. - It's also future-proof. As we add more features showing how to use different aspects of these generated libraries, we won't need to keep playing whack-a-mole with our stub. - Dependabot! Dependabot has been stripping the stuff in `packages` out of our `package-lock.json`, which we then need to re-add (see #126). If the stuff in `packages` is force-checked-in, we should work around this problem. - Less-treacherous contributing workflow: with the stub, anytime you pulled `stellar-scaffold-frontend` and ran `run dev`, you ended up overwriting the stub. This then led to ugly/confusing git changes that you needed to ignore. We should have _fewer_ such changes now, since the force-checked-in files _should_ match what's generated. Most of the time. When the contracts change, the generated packages will change too, and we will then end up with git diffs. But we actually WANT to check these in, to keep the described behavior here consistent. Cons ---- - In order to achieve all the Pros listed above, we should _not_ include a comment saying that these files are stubs. We should just leave them as-is, to avoid git diffs as much as possible. Given that the standard `scaffold init` workflow should result in these files *NOT* being included, though, this seems fine? And for people ignoring instructions and cloning `scaffold-stellar-frontend` directly, well, they'll have functioning packages and `src/contracts` imports. At least to start off. And then they'll change things and end up with confusing git diffs and seek support at that point. I feel like maybe the pros outweigh the cons? Wdyt? What else is in this commit =========================== - Tell eslint to ignore the temporary files in `target/packages`, since they were causing tremendous noise when linting locally [1]: #121 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pulling latest
mainand runningnpm icurrently changespackage-lock.json. Specifically, it adds the local packages from thepackagesfolder. Let's check in this change.