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

Generate output without namespace #32

Closed
wants to merge 8 commits into from
Closed

Generate output without namespace #32

wants to merge 8 commits into from

Conversation

abenhamdine
Copy link
Contributor

@abenhamdine abenhamdine commented Jan 11, 2017

Fixes #25

If namespace parameter is falsy (not provided or empty), don't generate anymore the namespace in the output file.

Sorry for the churn in artifact/osm.ts, I realized my indentation settings were different after having committed,

@abenhamdine
Copy link
Contributor Author

CI has failed, however on my Windows 10 machine, the test has failed too, but I see no difference between example/osm.ts and output artifacts/osm.ts (I compared the two files in VS Code, Compare contextual menu).

So I'm unable to see what to change to make the test pass.

@xiamx
Copy link
Contributor

xiamx commented Jan 11, 2017

Thanks for the patch!

Take a look at https://github.com/abenhamdine/schemats/pull/2 , I fixed the CI issue

@abenhamdine
Copy link
Contributor Author

abenhamdine commented Jan 11, 2017

Ah thx a lot, CI is green now 🎉

@xiamx
Copy link
Contributor

xiamx commented Jan 12, 2017

Nice! I'll merge this together with fix for #31 and release v1.0

@abenhamdine
Copy link
Contributor Author

IMHO a fix for #19 would be great for v1.0 (typescript users are sticklers for null checking these days 😉 )

@xiamx
Copy link
Contributor

xiamx commented Jan 12, 2017

IMHO a fix for #19 would be great for v1.0 (typescript users are sticklers for null checking these days 😉 )

Alright let's add that to the roadmap for v1.0 too . Let me know if you are pulling together a patch for any of those so we don't double the effort :)

@abenhamdine
Copy link
Contributor Author

Ok, I will give a try for #19 and #31 on Sunday.

@xiamx
Copy link
Contributor

xiamx commented Jan 16, 2017

Merged in here 64143dd

Thanks @abenhamdine

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.

None yet

2 participants