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

Fix <CompilerGeneratedFilesOutputPath> in Svg.Custom. #1153

Merged

Conversation

H1Gdev
Copy link
Contributor

@H1Gdev H1Gdev commented Apr 5, 2024

Reference Issue

Ref. #1141

What does this implement/fix? Explain your changes.

Because value of <CompilerGeneratedFilesOutputPath> is invalid, Generated directory is generated at root of execution environment.(ex: c:\Generated)

This PR will generate Generated under this project.

Any other comments?

Copy link
Member

@mrbean-bremen mrbean-bremen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - please add release notes. @paulushub - please have a look!

@paulushub
Copy link
Contributor

paulushub commented Apr 5, 2024

Thanks - please add release notes. @paulushub - please have a look!

Please, really sorry I am out - I always have something to occupy my time. But @inforithmics is in.
On the PR, take a look - I could not see any difference.
Sounds like simply replacing SvgSourcesBasePath with SvgRootPath to me.

@mrbean-bremen
Copy link
Member

Please, really sorry I am out - I always have something to occupy my time.

That is sad of course (that you are out of course, not that you have something to occupy you - if I don't have something left to occupy me, I'm probably dead 😀 ).
I added you as reviewer only because the original commit was yours, as a sanity check.

I'll wait for the release notes and merge in this case.

@H1Gdev
Copy link
Contributor Author

H1Gdev commented Apr 5, 2024

@mrbean-bremen

please add release notes.

Because it was a simple PR #1141, I thought it was not necessary.
Please check it because it has been added.

Sounds like simply replacing SvgSourcesBasePath with SvgRootPath to me.

We can understand by building...
I think it is annoying to other developers because it is generated in a place that is not temporary directory.

@paulushub
Copy link
Contributor

I think it is annoying to other developers because it is generated in a place that is not temporary directory.

What you documented is CompilerGeneratedFilesOutputPath, that should be a line fix.
What is the need in changing property name without documenting why? Such will only waste reviewer's time.
It took time to clear the PRs, hope you guys will make it work this time - good luck.

@mrbean-bremen
Copy link
Member

Because it was a simple PR #1141, I thought it was not necessary.

Only because the change was after the latest release, otherwise I would have agreed.

And while I approved this (too fast), the release notes are actually not saying what has changed in comparison with the latest release (e.g. the location of Generated), so I would ask you to change this.

@mrbean-bremen mrbean-bremen requested review from mrbean-bremen and removed request for paulushub April 6, 2024 16:10
@H1Gdev H1Gdev force-pushed the fix/CompilerGeneratedFilesOutputPath branch from b9466b7 to d3c5850 Compare April 6, 2024 23:49
Copy link
Member

@mrbean-bremen mrbean-bremen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mrbean-bremen mrbean-bremen merged commit 3bfa22a into svg-net:master Apr 7, 2024
8 checks passed
@H1Gdev H1Gdev deleted the fix/CompilerGeneratedFilesOutputPath branch April 7, 2024 08:39
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.

3 participants