-
Notifications
You must be signed in to change notification settings - Fork 37
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
feat(PoExtractor): add single file output #94
feat(PoExtractor): add single file output #94
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have few unit tests, there're many corner cases that we need to test, to ensure nothing is broken
src/OrchardCoreContrib.PoExtractor.Abstractions/LocalizableStringCollection.cs
Outdated
Show resolved
Hide resolved
…ingCollection.cs Co-authored-by: Hisham Bin Ateya <[email protected]>
Co-authored-by: Hisham Bin Ateya <[email protected]>
Co-authored-by: Hisham Bin Ateya <[email protected]>
Co-authored-by: Hisham Bin Ateya <[email protected]>
Co-authored-by: Hisham Bin Ateya <[email protected]>
Look at this commit It's a preview of some kinds of test we can do. |
Thanks, @VincenzoCarlino, we might need to test more cases such as if there's a PO file with the same specified file |
Yes @hishamco , this is just an example of the structure of tests. Tomorrow I'll add more tests. |
Please fix the build |
Hi @hishamco , I think that we don't have an easy way to test output files... using the real filesystem in the pipeline is not a good option. Do you have any suggestions? |
Let me try .. |
Ok @hishamco I'll wait for your commit, let me know if I can do something. |
Sorry for the late, I will struggle to have a look today because we need to finalize this and publish a release before Orchard Core 2.0.0 |
Hello, any update? |
Seems I forgot this one, let me check it tonight if it's possible. My plan to publish a release ASAP and will be a part of OC 2.0.0 |
I suggest trying this PR from |
@VincenzoCarlino please remove the changes in the test project, then I will merge, this not take so long to merge |
Thanks for your contribution @VincenzoCarlino |
Implementation of: #93