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

feat: add a placeholder README file in codegen'd directory #774

Merged
merged 6 commits into from
Oct 20, 2023

Conversation

kanadgupta
Copy link
Member

🚥 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
@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.

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
Member 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
5 checks passed
@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.

2 participants