Skip to content

Conversation

@kanadgupta
Copy link
Contributor

🚥 Resolves RM-8188

🧰 Changes

This PR enhances our SDK generation to automatically create placeholder README.md files in the codegen'd directories.

As part of this, I made a few test enhancements so the UPDATE_FIXTURES env variable will now create any missing fixtures.

🧬 QA & Testing

Does the copy look good? And do tests pass?

@kanadgupta kanadgupta added enhancement New feature or request area:core Issues related to `core`, which is the package that powers the SDKs at runtime labels Oct 19, 2023
@kanadgupta kanadgupta marked this pull request as ready for review October 19, 2023 22:22
@kanadgupta kanadgupta requested a review from erunion October 19, 2023 22:22
@erunion erunion added this to the v7 milestone Oct 19, 2023
Here's some additional info about the generated SDK:
\`api\` version: ${PACKAGE_VERSION}
Generated at ${new Date().toISOString()}
Copy link
Member

Choose a reason for hiding this comment

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

After #775 is merged in you should be able to grab the createdAt date from the lockfile for this with this code:

storage.getFromLockfile().createdAt

Copy link
Member

Choose a reason for hiding this comment

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

should be able to pull this in now that createdAt has been added to the lockfile.

@kanadgupta kanadgupta requested a review from erunion October 20, 2023 14:09
Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

sick

*/
createREADME() {
let createdAt = new Date().toISOString();
const currentAPI = Storage.getLockfile().apis.find(api => api.identifier === this.identifier);
Copy link
Member

Choose a reason for hiding this comment

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

This is duplicating work in the getFromLockfile storage instance method but this whole storage class is really becoming a bit of a pain with its mix of instance and static methods. You don't have a great way of accessing an instance of `storage though here so it's fine.

Maybe the whole storage class should just be refactored to be entirely static. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i noticed that getFromLockfile method but wasn't able to get it working without some deeper refactors 😮‍💨

Maybe the whole storage class should just be refactored to be entirely static. 🤔

into this idea!

@kanadgupta kanadgupta merged commit 3bbb7a8 into main Oct 20, 2023
@kanadgupta kanadgupta deleted the kanad/rm-8188-create-readmemd-in-codegend-directory branch October 20, 2023 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core Issues related to `core`, which is the package that powers the SDKs at runtime enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants