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

Change WiX convert error messages to information messages where appropriate #97

Closed
wants to merge 1 commit into from
Closed

Conversation

MarkStega
Copy link
Contributor

No description provided.

src/wix/wix/Properties/launchSettings.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@MarkStega
Copy link
Contributor Author

MarkStega commented Jan 20, 2022

  1. For some reason, after getting an update to VS2022 17.1 preview there was an issue finding the .Net 6.0.101 SDK during the build; I added a global.json file to the root that excludes pre-release sdk's

  2. The readme,md is updated to have the start of 'Developing WiX' information

  3. WixConverter.cs now has a method 'OnInformation'. It displays an infomation level message prefixed with [Converted] to indicate that the construct in question was updated. I changed the 'Errors' member to 'Messages' but did not go through the test fixtures to change the name of the locals. I will if desired. The methodology of deciding whether to replace OnError with OnInformation was that if a transform was performed then OnInformation was appropriate.

@MarkStega MarkStega marked this pull request as ready for review January 20, 2022 18:20
@rseanhall
Copy link
Contributor

  1. We generate a global.json in src which already excludes prerelease versions: https://github.com/wixtoolset/wix4/blob/212a7179408758b27fea5a12274f739a5205b2d7/src/internal/SetBuildNumber/SetBuildNumber.proj#L41
    We haven't typically been interested in spending the time to support preview versions of VS or .NET Core, but there have been breaking changes in the past that we would have noticed earlier if someone had been brave enough to use the preview version. We would be interested in the details of this problem before solving it this way, and would prefer to do that in a separate PR.

  2. We would prefer to take this in a separate PR so that the doc changes and conversion changes can be reviewed and merged independent of each other.

@MarkStega
Copy link
Contributor Author

MarkStega commented Jan 20, 2022

Sean,

Not a problem -- I dropped the global.json from this PR and will do a build demonstrating what I saw that lead me to the addition of same. I am building using the release version of VS2022.

I also moved the documentation changes to PR #99

Initial WiX information messages


Added global.json to avoid interference from pre-release sdk, removed launchsettings.json, changed the coverter to use Messages rather than Errors


Fix typo


Remove global.json
Typo


Revert changes to readme.md
@robmen
Copy link
Member

robmen commented Mar 5, 2022

Sorry, I missed (or didn't get) the notice that this PR was out of draft. I've rebased it to current and did a bit of clean-up in PR #135.

Thanks for doing the work to bring this about.

@rseanhall
Copy link
Contributor

I don't think I've ever gotten a notification that a PR was marked ready for review. I would have expected that review request to have generated a notification, though.

@robmen robmen closed this in #135 Mar 5, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 5, 2022
@MarkStega MarkStega deleted the WiXConvertInfo branch March 8, 2022 22:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants