hamlet-io / executor-bash

Executor for hamlet based on bash
GNU General Public License v3.0
0 stars 4 forks source link

refactor: remove dos2unix #286

Closed roleyfoley closed 2 years ago

roleyfoley commented 2 years ago

Intent of Change

Description

Motivation and Context

The dos2unix package is not installed in a lot of machines and adds an additional requirement to setting up hamlet. Its use has been limited and tools like git manage line endings anyway. Removing this from hamlet itself reduces the complexity in setting up hamlet

How Has This Been Tested?

Tested locally

Related Changes

Prerequisite PRs:

Dependent PRs:

Consumer Actions:

roleyfoley commented 2 years ago

Just felt like quite an edge case that could be handled outside of hamlet. At the current stage you need to ensure you always have dos2unix installed whenever you want to run any hamlet command.

I think the manageJSON.sh usage is the main culprit there so we could leave it in manage crypto. Still feels like it could be managed as an outside process as part of a custom Pipeline or process though

ml019 commented 2 years ago

OK - so let's go with removing it and see what cases come up as a result and make a decision then on how to handle them. Given we require python to be installed, another option would be a simple python script.