fix!: remove game terminology #45
Conversation
|
I was reading this repo and I thought to myself- "these references to game1 could use some updating" then boom you put this PR up. Thank you! |
CONTRIBUTING.md
Outdated
| @@ -8,45 +8,103 @@ yarn test | |||
|
|
|||
| ``` | |||
| $ yarn test | |||
There was a problem hiding this comment.
We already have yarn test above, maybe remove that one?
| @@ -3,7 +3,7 @@ import { E } from '@endo/far'; | |||
| import { makeMarshal } from '@endo/marshal'; | |||
There was a problem hiding this comment.
Nit:
offer-up-proposal.js
offer-up.contract.js
Naming here seems inconsistent. Is there a convention we want to use?
There was a problem hiding this comment.
@turadg suggested xyz.contract.js at some point when we were talking about using almost-but-not-quite jessie rules for linting contracts. The idea is growing on me... I find myself wishing I could see which are the bundle roots at a glance now and then; so since I had to rename gameAssetContract.js anyway, I thought I'd go for it.
(neaby: @turadg also pointed out test-foo.js in our unit testing norms is counter to ava norms, which call for foo.test.js)
in swaparoo, @dtribble did some renaming so that you could just set the contract name in one place; in particular, instead of build-game1-start.js , he used build-contract-deployer.js . The name "deploy" is growing on me too. After all: the proposal is what goes out to the BLD stakers; the code here is what executes if the proposal carries. I'm less convinced about filenames that are contract-agnostic. I have multiple contracts in a package more often than not.
So if I'm doing any more renaming in this PR (and given the amount of manual testing, I am not excited about it) it will probably be to change offer-up-proposal.js to offer-up.deploy.js. But then... startOfferUpContract is the main function in there, which argues for offer-up.start.js... or something.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "name": "game-places", | |||
| "name": "offer-up", | |||
| "version": "0.1.0", | |||
There was a problem hiding this comment.
Should we manually bump the version? Your PR title denotes a breaking change but I'm not sure CI is set up to care about that.
There was a problem hiding this comment.
The breaking change indicator is because the contract API surface changed in a way that broke the ui (along with any other clients that may exist).
As to versions... I try not to learn any more npm version stuff than I have to. I have not been forced to learn enough to indicate that the version should change here. The machines have not complained.
- rename build-game1-start - strike "game" from test-contract.js, CONTRIBUTING.md - update repo URLs - strike "game" - strike "game" from package names - avoid "game" in contract filename - rename contract instance, brand, keywords - avoid "game" in ui, incl invitation maker
fixes #22
I'd like somebody else to run thru the whole getting started flow with this.
@kbennett2000 we'll want to re-shoot the video eventually. You could do that as a form of review.
@0xpatrickdev I did a bunch of manual testing. More on that in separate issues...