Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Make sure we test all the local modules #2822

Merged
merged 2 commits into from
Feb 5, 2020

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Feb 5, 2020

This, in particular includes the new pkg/install module

Followup from #2778

@2opremio 2opremio requested a review from squaremo February 5, 2020 11:35
@2opremio 2opremio added this to the 1.18.0 milestone Feb 5, 2020
@hiddeco
Copy link
Member

hiddeco commented Feb 5, 2020

@2opremio if the amount of time you have available allows you to, can you please port this to the Helm operator? (If not, please tell and I will find the time to do it myself).

@2opremio
Copy link
Contributor Author

2opremio commented Feb 5, 2020

Sure! Let's get it merged first though.

@2opremio 2opremio requested a review from hiddeco February 5, 2020 12:00
@2opremio
Copy link
Contributor Author

2opremio commented Feb 5, 2020

It's worth noting that, due to the (overly) sophisticated way we calculate build dependencies, the build wasn't impacted by this.

That said, I think we should simplify the build dependency calculation since it takes a long time, probably longer than invoking go systematically (letting it figure out if anything changed). Regardless, it's a pain when using $ make ... shell autocompletion (it takes seconds right now).

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

After translating some of the make voodoo this does LGTM

Makefile Outdated Show resolved Hide resolved
@squaremo
Copy link
Member

squaremo commented Feb 5, 2020

That said, I think we should simplify the build dependency calculation since it takes a long time

Yes, it probably takes as long as just invoking go build and letting go decide.

@2opremio 2opremio force-pushed the test-pkg-install-again branch from 9738e37 to a2fcb7a Compare February 5, 2020 12:16
@2opremio
Copy link
Contributor Author

2opremio commented Feb 5, 2020

Yes, it probably takes as long as just invoking go build and letting go decide.

Then I think I will make the build targets PHONY and invoke go systematically. I will do that in a separate PR.

@2opremio 2opremio merged commit be3c7cb into fluxcd:master Feb 5, 2020
@2opremio 2opremio deleted the test-pkg-install-again branch February 5, 2020 12:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants