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

Package config with dynamic package name processing + more #14

Closed
wants to merge 38 commits into from

Conversation

gkocak-scottlogic
Copy link
Collaborator

@gkocak-scottlogic gkocak-scottlogic commented Mar 1, 2023

This PR is similar to #9 but includes the post-processing step to generate the dynamic folder structures.

Warning: This PR currently relies on a PR in the main forge (ScottLogic/openapi-forge#172). Merging this PR is not recommended until the said PR on main forge is merged.


Additionally, thanks to the post-processing, it is possible to split the model into seperate files without altering the forge generation structure. This PR also includes a temporary way of dealing with such situation.

config.json Outdated Show resolved Hide resolved
postProcess.js Outdated Show resolved Hide resolved
postProcess.js Outdated Show resolved Hide resolved
const shell = require("shelljs");
const model_fix = require("./postProcessModel");

module.exports = (folder, _, options) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Were we going to put formatting under the banner of post-processing? This would have the benefit of the main forge only needing to know about post-processing in general, not formatting. Although this would need changes to the other generators, I don't think those changes will be very significant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that formatting is actually a post-processing stage and it would make sense every language to do their formating in this stage. And I also agree that it's not a big change either. Maybe we can discuss this change over the demo meeting.

Copy link
Contributor

Choose a reason for hiding this comment

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

I realise that we didn't end up chatting about this. Did you want to message Colin about this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True, we forgot to chat about this. And since the post-processing is already merged at the main repo alongside with formatting, we can keep the logic as is in this PR at least. I can open a ticket/issue to track the merge of formatting into post-processing.

@gkocak-scottlogic
Copy link
Collaborator Author

check #22

@gkocak-scottlogic gkocak-scottlogic mentioned this pull request Mar 14, 2023
@gkocak-scottlogic gkocak-scottlogic deleted the feat/package-postprocess branch April 4, 2023 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants