Skip to content

add python version requirement to BUILDING.md#256

Closed
briaguya0 wants to merge 2 commits intoHarbourMasters:developfrom
briaguya0:patch-1
Closed

add python version requirement to BUILDING.md#256
briaguya0 wants to merge 2 commits intoHarbourMasters:developfrom
briaguya0:patch-1

Conversation

@briaguya0
Copy link
Contributor

extract_assets uses match statements which were added in python version 3.10

extract_assets uses match statements which were added in python version 3.10
@briaguya0
Copy link
Contributor Author

I know the link is to the windows version of python 3.10, but people who already have and older version of python installed may skip past it not knowing they need a newer version

@MegaMech
Copy link
Contributor

MegaMech commented May 2, 2022

#251 removes this requirement. putting this to python 3.6 would be good and have this PR sneak in behind it.

@MegaMech
Copy link
Contributor

MegaMech commented May 2, 2022

Could we actually change this line to:

Requires [Python](link) >= 3.6.

Seems cleaner to me

@briaguya0
Copy link
Contributor Author

Requires Python >= 3.6.

sounds good. to confirm: requires instead of install?

@MegaMech
Copy link
Contributor

MegaMech commented May 2, 2022

I think so, as you said, some people already have it. There's not necessarily or right or wrong way to do this and requires makes more sense than install to me. I think it just makes sense grammatically. Perhaps someone else has a differing opinion?

It's clean, gets to the point without extra words or brackets.

#251 removes the 3.10 specific match statements, dropping the python version requirement to 3.6.
Copy link
Contributor

@MegaMech MegaMech left a comment

Choose a reason for hiding this comment

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

Needs to be merged after #251

@MegaMech
Copy link
Contributor

MegaMech commented May 8, 2022

Hi briaguya-ai,

I have a bit of an odd request. Could you please PR this to my fork at the below PR?

#262

This would help SoH for reviewing the current readme changes all-together and later merging the readme with the linux PR.

Thanks!

@briaguya0
Copy link
Contributor Author

closing and using MegaMech#2 instead

@briaguya0 briaguya0 closed this May 8, 2022
aMannus pushed a commit to aMannus/Shipwright that referenced this pull request Jul 2, 2022
…ndo-menu-issue

make sure to actually init the cvar
Malkierian pushed a commit to Malkierian/Shipwright that referenced this pull request Nov 20, 2023
Co-authored-by: briaguya <briaguya@alice>
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.

2 participants