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

updated Bullseye from 4.0.0 to 4.2.0 #735

Merged
merged 1 commit into from Nov 10, 2022
Merged

updated Bullseye from 4.0.0 to 4.2.0 #735

merged 1 commit into from Nov 10, 2022

Conversation

adamralph
Copy link
Contributor

Welcome!

Thanks for your interest in contributing to this project. Any contribution will
be gladly accepted, provided that they are generally useful and follow the
conventions of the project.

  1. Please create one pull request for each feature. This results in smaller pull requests that are easier to review and validate.

  2. Avoid reformatting existing code unless you are making other changes to it.

    • Cleaning-up of usings is acceptable, if you made other changes to that file.
    • If you believe that some code is badly formatted and needs fixing, isolate that change in a separate pull request.
  3. Always add one or more unit tests that prove that the feature / fix you are submitting is working correctly.

  4. Please describe the motivation behind the pull request. Explain what was the problem / requirement. Unless the implementation is self-explanatory, also describe the solution.

    • Of course, there's no need to be too verbose. Usually one or two lines will be enough.
  5. Follow the project's coding conventions


In Bullseye 4.2.0, Palette and HostExtensions are logically public (they exist in the Bullseye namespace), so there's no longer a need to use anything from the Bullseye.Internal namespace to provide output which matches the coloring of, and symbols used by, Bullseye output. Also, Bullseye.Internal.OperatingSystem has been removed in favour of System.Runtime.InteropServices.OSPlatform.

@adamralph adamralph mentioned this pull request Oct 30, 2022
9 tasks
@adamralph
Copy link
Contributor Author

@aaubry I wanted to test the beta against your code, and it seems to work well. I'll move forward to RC and/or RTM soon. Please let me know if you think there should be any further changes.

@EdwardCooke
Copy link
Collaborator

@adamralph let us know when your new version is complete and we’ll updates. Thanks for your help with this project.

@adamralph adamralph changed the title updated Bullseye from 4.0.0 to 4.2.0-beta.1 updated Bullseye from 4.0.0 to 4.2.0 Nov 10, 2022
@adamralph
Copy link
Contributor Author

adamralph commented Nov 10, 2022

@aaubry @EdwardCooke I've updated this PR to upgrade to Bullseye 4.2.0 (RTM).

@EdwardCooke EdwardCooke merged commit eb821bf into aaubry:master Nov 10, 2022
@EdwardCooke
Copy link
Collaborator

Awesome. Thanks for the contribution.

@adamralph adamralph deleted the bullseye-public-only branch November 10, 2022 11:52
@aaubry
Copy link
Owner

aaubry commented Dec 2, 2022

I had to revert to Bullseye 4.1.1 because version 4.2.0 was always detecting the host as AppVeyor.
This build project is very annoying to debug because some tests need to be run on the actual build host and that takes a lot of time. I'd prefer if we didn't touch this part unless there is something that is not working or a new feature is needed. Updating libraries just for the sake of running the latest version is not a good use of our time.

@adamralph
Copy link
Contributor Author

@aaubry thanks for letting me know about the bug. I've released a fix in 4.2.1-rc.1.

I'd prefer if we didn't touch this part unless there is something that is not working or a new feature is needed. Updating libraries just for the sake of running the latest version is not a good use of our time.

No problem—I won't raise any unsolicited PR's like this in the future. Have you considered stating this preference in your PR template?

BTW, FWIW, bear in mind that you are currently using the Bullseye.Internal namespace which is subject to change, without notice, in any version. The comes at the risk of making it difficult to upgrade to some future version of Bullseye when you want to. By upgrading to Bullseye 4.2.1 (an RTM will available in the next few days after the RC has proven to fix the bug), you will no longer need to use the Bullseye.Internal namespace and you can remove that risk.

I'd be happy to raise another PR, similar to this one, to upgrade to 4.2.1, when the RTM is available, but I won't do so unless you want me to, given your preference stated above. If you'd like me to raise that PR then just let me know.

@adamralph adamralph mentioned this pull request Dec 3, 2022
9 tasks
@aaubry
Copy link
Owner

aaubry commented Dec 3, 2022

@adamralph I apologize if my previous comment was too harsh. I was tired and after spending many hours debugging this problem and didn't express myself in the most correct way. I should have tested the changes more thoroughly before merging the pull request.
Of course you are right about keeping our dependencies updated. By all means, if you are willing to raise a PR when the next version is available, I will be happy to validate it.

Thank for your hard work.

@adamralph
Copy link
Contributor Author

@aaubry understood. No hard feelings. The bug was a silly oversight by me. I even have smoke test CI builds running on various CI providers to ensure hosts are detected correctly and I failed to check them before releasing 4.2.0. This was a good reminder to do that.

I'll send a PR as soon as I release 4.2.1 RTM.

@aaubry
Copy link
Owner

aaubry commented Dec 5, 2022

This feature has been released in version 12.1.0.

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.

None yet

3 participants