#1261 Small build fixes

Merged
max.mehl merged 1 commits from small-build-fixes into master 1 year ago
max.mehl commented 1 year ago
Owner

Fixes some errors I discovered when setting up a new build server.

I am not so happy with the lessc check as it duplicates the regular dependency check above. However, it has to happen later in the file since arguments.sh is just run there. Only this evaluates the build_env parameter.

@reinhard, do you have a better idea? Could we run arguments.sh earlier?

Fixes some errors I discovered when setting up a new build server. I am not so happy with the lessc check as it duplicates the regular dependency check above. However, it has to happen later in the file since arguments.sh is just run there. Only this evaluates the build_env parameter. @reinhard, do you have a better idea? Could we run arguments.sh earlier?
max.mehl added the
build
label 1 year ago
reinhard commented 1 year ago
Collaborator

I could think of 3 options to avoid that duplication, sorted by my personal preference:

  1. Check for lessc unconditionally.
  2. Take the existence of realpath, find, [e]grep, sed, tee, and date for granted, in which case the whole dependency check could be moved after the argument parsing.
  3. Put the duplicated part into a shell function.
I could think of 3 options to avoid that duplication, sorted by my personal preference: 1. Check for lessc unconditionally. 2. Take the existence of realpath, find, [e]grep, sed, tee, and date for granted, in which case the whole dependency check could be moved after the argument parsing. 3. Put the duplicated part into a shell function.
max.mehl commented 1 year ago
Poster
Owner

Thanks for the feedback!

I went with the function because lessc is a heavy program which I wouldn't want everyone to have installed, and take things as granted proved to be not the most successful path sometimes ;)

It's still not too complex to maintain and understand IMHO

Thanks for the feedback! I went with the function because lessc is a heavy program which I wouldn't want everyone to have installed, and take things as granted proved to be not the most successful path sometimes ;) It's still not too complex to maintain and understand IMHO
max.mehl closed this pull request 1 year ago
The pull request has been merged as b83e8a1d17.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.