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

Convert install scripts to python #89

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

AgentK9
Copy link

@AgentK9 AgentK9 commented Aug 24, 2023

Python is more universal than perl and allows for better extensibility and portability, not to mention readability.

@mstorsjo
Copy link
Owner

mstorsjo commented Sep 9, 2023

I'm somewhat ambivalent about this PR. (I haven't studied your suggested code in detail yet.)

Yes, python is probably to prefer over perl for the fixinclude and lowercase scripts. But for the install.sh script, I'm not really sure about whether this is a net win or loss - what do we gain from converting that to python?

(The actions run that I just enabled will probably fail; this is due to a tool regression that I've worked around on master; if you rebase your branch on latest master, it should run successfully again.)

@AgentK9
Copy link
Author

AgentK9 commented Sep 9, 2023

Hello,

Thanks so much for your time in reviewing this. I have merged main into my branch. If you'd like, I can also squash all of these commits.

Here are a few reasons why the install.sh script should be changed along with the others:

  1. Unified Codebase: Aligns with the existing Python components, streamlining maintenance.
  2. Inclusivity: Facilitates contributions from Windows developers unfamiliar with GNU shell/sh.
  3. Versatility: Enhances portability and allows for future feature integrations with ease.

I'd really appreciate any feedback you have.

@SaifRushdHadad
Copy link

I'm somewhat ambivalent about this PR. (I haven't studied your suggested code in detail yet.)

@mstorsjo, I was wondering if you had reviewed the suggested changes? I know it has been some time since the PR was submitted but rebasing and updating should be doable.

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