Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
matthewvolk
left a comment
There was a problem hiding this comment.
A couple of notes to consider for the future, but this looks good. Thanks for switching to the try/catch pattern to bubble up errors 🙏
| }); | ||
|
|
||
| describe('bundle zip generation', () => { | ||
| describe('loadProjectId', () => { |
There was a problem hiding this comment.
🍹 Might be able to ditch this set of tests if we switch to conf
aa69177 to
b3d26e2
Compare
3012ce8 to
97c21b0
Compare
97c21b0 to
cbb7026
Compare
| getProjectId(): string { | ||
| const projectId = this.config.get('projectId'); | ||
|
|
||
| if (!projectId) { | ||
| throw new Error( | ||
| 'No `projectId` found in .bigcommerce/project.json. Please add a valid `projectId` (UUID) to your bigcommerce configuration file.', | ||
| ); | ||
| } | ||
|
|
||
| return projectId; | ||
| } |
There was a problem hiding this comment.
🍹I wonder if we'd be able to do something like get(field: keyof ProjectConfigSchema) for a future where we might want to store more keys in project.json. Not necessary right now though!
There was a problem hiding this comment.
Please add a valid
projectId(UUID) to your bigcommerce configuration file.
Can we just name the field projectUuid so we don't need to mention it's a uuid?
There was a problem hiding this comment.
I added generic get/set. I have also swapped the error to be more generic.
But now that you mentioned it, I'm fine with renaming projectUuid, should we use this throughout the CLI tho? Including command options, etc?
There was a problem hiding this comment.
Good catch - I'm down to switch the options to --project-uuid, and anywhere else it pops up
539b2f6 to
336902f
Compare
7983a10 to
8269f96
Compare
What/Why?
project_uuidandupload_uuid.Still missing: poll after deploy
Testing
Unit tests + locally

Migration
N/A