Skip to content

Fixed Issue #193#195

Merged
shreys7 merged 4 commits intopostmanlabs:developfrom
nikhilmuz:issue-193
Mar 19, 2020
Merged

Fixed Issue #193#195
shreys7 merged 4 commits intopostmanlabs:developfrom
nikhilmuz:issue-193

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