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

Add DateOnly support. #839

Merged
merged 2 commits into from
May 27, 2022
Merged

Add DateOnly support. #839

merged 2 commits into from
May 27, 2022

Conversation

CollinAlpert
Copy link

@CollinAlpert CollinAlpert commented May 22, 2022

This PR adds support for setting cells through a DateOnly value.

@CollinAlpert
Copy link
Author

It seems the build is failing because it uses .NET 5. .NET 5 is end-of-life and not supported anymore. Will it be possible to upgrade to .NET 6, the LTS release?

@tonyqus
Copy link
Member

tonyqus commented May 22, 2022

I've fixed the build issue. Sorry, I'm too lazy to get it fixed.

@CollinAlpert
Copy link
Author

Build looks good. How does the PR itself look?

@tonyqus
Copy link
Member

tonyqus commented May 22, 2022

Since you changed the ICell interface, please change XSSFCell and SXSSFCell as well. They both implements ICell interface.

I'm curious why there is no compilation error if XSSFCell and SXSSFCell doesn't have SetCellValue(DateOnly value) method. Maybe a bug of .NET 6? I'm not sure

@CollinAlpert
Copy link
Author

I was also surprised to see the build didn't fail. I assume it is because the project is built using .NET Standard and due to the preprocessor directives, the method gets hidden. I added the methods in the other implementations as well now.

@CollinAlpert
Copy link
Author

How does it look? Can we merge this?

@tonyqus
Copy link
Member

tonyqus commented May 27, 2022

LGTM

@tonyqus tonyqus merged commit 57d5b57 into nissl-lab:master May 27, 2022
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