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

(GH-1364) - update template service to generate UTF8 files without BOM. #1409

Closed

Conversation

Benny1007
Copy link

Updated TemplateService.cs to use UTF8Encoding with no BOM when creating the template files in relation to #1364

Not sure why other files were updated, this is my first ever pull request.

@TheCakeIsNaOH
Copy link
Member

@Benny1007 Are you still interested in completing this?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Benny1007
Copy link
Author

i can't even remember this, is it still required?

@AdmiringWorm
Copy link
Member

@Benny1007 IMO, it is still a valid contribution as files are even being generated with BOM.
It looks like the PR would need to be cleaned up, though, and at least the PowerShell files you have changed to not generate with BOM needs to have BOM (otherwise PowerShell will read them as ASCII).

However, PR #2064 perhaps could supersede this one as it is also up to date.

@ferventcoder
Copy link
Member

I think we could go for #2064 to supersede this - there were some files in here that did not appear to need to be adjusted that needed to be pulled out. It also doesn't appear that the CLA was signed (CLAHub the older one is showing as not signed, but it could have been at the time this was submitted). Thoughts?

@TheCakeIsNaOH
Copy link
Member

@gep13 This should be able to be closed as #2064 has been merged

@gep13
Copy link
Member

gep13 commented Apr 16, 2021

@TheCakeIsNaOH makes sense to me. Thanks for pointing this out!

@gep13 gep13 closed this Apr 16, 2021
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.

6 participants