Skip to content

Fixed Issue #193 #195

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

Merged
merged 4 commits into from
Mar 19, 2020
Merged

Fixed Issue #193 #195

merged 4 commits into from
Mar 19, 2020

Conversation

nikhilmuz
Copy link
Contributor

Refactored global namespase import for shelljs in boilerplate and refactored codegens accordingly as per issue #193

@nikhilmuz nikhilmuz changed the title Trying to fix Issue #193 Fixed Issue #193 Mar 4, 2020
@nikhilmuz
Copy link
Contributor Author

Fixed global namespace imports for shelljs having potential for conflict with local functions in future and refactored all codegens respectively. Also fixed it for boilerplate in order to avoid this while bootstrapping for new language/framework. @umeshp7 Approval required.

@nikhilmuz
Copy link
Contributor Author

@shreys7 Review required

@shreys7
Copy link
Member

shreys7 commented Mar 18, 2020

@nikhilmuz I have reviewed the code. Can you just check once if the following commands work perfectly after the changes?

in the root repo:

  • npm run test
  • npm run test <codegen-name>

cd inside any codegen

  • npm run test
  • npm run test-unit
  • npm run test-lint

@nikhilmuz
Copy link
Contributor Author

nikhilmuz commented Mar 18, 2020 via email

@nikhilmuz
Copy link
Contributor Author

@nikhilmuz I have reviewed the code. Can you just check once if the following commands work perfectly after the changes?

in the root repo:

  • npm run test
  • npm run test <codegen-name>

cd inside any codegen

  • npm run test
  • npm run test-unit
  • npm run test-lint

@shreys7 I have done lint test on all the codegens locally npm run test-lint and since CI is passing and while going through CI log and test script I checked it runs npm run test recursively on each codegen. Thus, the codegen must be passing. So it seems safe to merge this PR for the changes which is reflected here.
P. S. All the npm test not passing on my local as I have not configured this repo for all languages. Though, I don't guess it is something to be concerned about as it's passing on CI.

@shreys7
Copy link
Member

shreys7 commented Mar 19, 2020

Merging this, thanks for the contribution 😄

@shreys7 shreys7 merged commit 60f65c4 into postmanlabs:develop Mar 19, 2020
@nikhilmuz nikhilmuz deleted the issue-193 branch March 20, 2020 06:56
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