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

Issue1054 #100

Merged
merged 3 commits into from Apr 4, 2022
Merged

Issue1054 #100

merged 3 commits into from Apr 4, 2022

Conversation

PatrickFerber
Copy link
Member

No description provided.

CHANGES.md Outdated
@@ -11,6 +11,10 @@ after the corresponding tracker issues.

## Changes since the last release

- infrastructure: Upgrade GitHub Actions to Windows Server 2019
(Visual Studio Enterprise 2019) and Windows Server 2022 (Visual Studio
Enterprise 2022).
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add "Remove Windows Server 2016 because GitHub no longer supports it."

Copy link
Member Author

Choose a reason for hiding this comment

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

In our requirements (https://www.fast-downward.org/ForDevelopers/Requirements?highlight=%28Windows%29)
we state that we support the latest two windows versions from GitHub Actions. Thus, just stating that we upgrade (to follow our guidelines for versions) would be sufficient for me. But if you want, I can explicitly state that 2016 became deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

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

"ForDevelopers" is means as internal documentation. It's good to be explicit about this. Also, didn't it became unavailable? "Deprecated" just means that you're encouraged to no longer used it because it's expected to go away in the future.

@@ -20,7 +20,7 @@ This version of Fast Downward has been tested with the following software versio
| Ubuntu 20.04 | 3.8 | GCC 9, GCC 10, Clang 10, Clang 11 | 3.16 |
| Ubuntu 18.04 | 3.6 | GCC 7, Clang 6 | 3.10 |
| macOS 10.15 | 3.6 | AppleClang 12 | 3.19 |
| Windows 10 | 3.6 | Visual Studio Enterprise 2017 (MSVC 19.16) and 2019 (MSVC 19.28) | 3.19 |
| Windows 10 | 3.6 | Visual Studio Enterprise 2019 (MSVC 19.29) and 2022 (MSVC 19.31) | 3.22 |
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you verified that Python version 3.6 is still correct for both Windows versions? It seems unusual to have no Python minor version advance in six years, from 2016 to 2022.

Copy link
Member Author

Choose a reason for hiding this comment

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

Their runner uses natively python3.9, but we explicitly install python3.6, thus, we can still use it. We state that we test the default python version of the latest two Ununtu versions. Once Ubuntu 22 is out, we need to update the Python versions, I expect.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to test the "default" Python version of each platform (plus possibly more for Ubuntu). For Windows I think we should test the version that is recommended by Python itself, which is 3.10 currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me. But Patrick already merged this issue, so if someone wants to change this, we should open a new issue in which we update the Python version and test with the newer version. If nobody feels strongly enough about this to work on it, I'm also OK with sticking to Python 3.6 for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, no need to switch now.

@PatrickFerber PatrickFerber merged commit 8c96f04 into aibasel:main Apr 4, 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
3 participants