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

Delete unnecessary complexity from build-lf-cli #1745

Merged
merged 1 commit into from May 18, 2023
Merged

Conversation

petervdonovan
Copy link
Collaborator

I have not been able to get this script to detect changes in my file system. At first I thought this was a Gradle issue, but it is just a problem in the bash script.

If anyone wishes to make this mechanism work properly, then I will not stop them. However, Gradle is already supposed to detect when nothing needs to be rebuilt, so if we do it this way and something needs to be rebuilt, then we are just doing the same check twice, which is a waste of time. Furthermore, if nothing needed to be rebuilt, then we would not be running this script. Let us not make an incremental build system out of bash scripts.

@petervdonovan petervdonovan requested a review from lhstrh May 18, 2023 05:18
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

I see no problem with this and your argument makes sense. I'd like @cmnrd to have a look at it though.

@lhstrh lhstrh requested a review from cmnrd May 18, 2023 05:21
Copy link
Collaborator

@cmnrd cmnrd left a comment

Choose a reason for hiding this comment

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

I agree that we should let Gradle decide what needs to be rebuilt. Thanks for the cleanup!

@cmnrd cmnrd merged commit 9e075e9 into master May 18, 2023
40 of 41 checks passed
@cmnrd cmnrd deleted the do-not-skip-rebuilding branch May 18, 2023 08:10
@petervdonovan petervdonovan added the refactoring Code quality enhancement label Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code quality enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants