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

feature(kamel): Validate --context when given #423

Merged
merged 1 commit into from
Feb 13, 2019

Conversation

rhuss
Copy link
Contributor

@rhuss rhuss commented Feb 12, 2019

No description provided.

@rhuss rhuss force-pushed the pr/camel-k-validation-fix branch 2 times, most recently from b75931c to a2c8b27 Compare February 12, 2019 18:42
pkg/cmd/install.go Outdated Show resolved Hide resolved
it.in.headers['RandomValue'] = rnd.nextInt()
}
.to('log:info?showHeaders=true')
name: routes.groovy
Copy link
Contributor

Choose a reason for hiding this comment

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

May this be changed by the fact that your pr is created from an old commit ?

Copy link
Contributor Author

@rhuss rhuss Feb 13, 2019

Choose a reason for hiding this comment

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

Not sure. I added a sorting option to ls when creating that file previously, but it seems that the PR has not been taken over. Actually it then depends on the way how ls works on different systems how resources are picked up. My ls uses alphabetical sort order by default, so orders 'cr-example.yml' after 'camel-catalog-2.23.1.yaml' and before 'crd-camel-catalog.yaml'.

In this file frommaster 'cr-example.yml' comes right before 'operator-deployment-kubernetes.yml' which seems wrong if you assume alphabetical ordering.

I'd recommend to add a "| sort |" after ls to not depend on platform default for "ls"'s ordering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's here where I would add a | sort in between.

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure what appended but I see this commit

https://github.com/apache/camel-k/commit/c2053bfe2155bb1e418457840434d8b07bde6985

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, there is already a sort at the end.

So no idea how this file was regenerated the last time.

Copy link
Contributor

Choose a reason for hiding this comment

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

so looks like there are some differences between linux and osx implementations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so as this final | sort works the same on linux and osx. No idea though why the resource list checked in in master is not sorted.

I suggest to merge this in (as its sorted now), and check for a PR which changes this again into a non-sorted order.

@rhuss rhuss force-pushed the pr/camel-k-validation-fix branch 2 times, most recently from 5c020e5 to cadad01 Compare February 13, 2019 08:47
@rhuss
Copy link
Contributor Author

rhuss commented Feb 13, 2019

Test failure is unrelated to this change:

The job exceeded the maximum log length, and has been terminated.

@lburgazzoli
Copy link
Contributor

working on it

@rhuss
Copy link
Contributor Author

rhuss commented Feb 13, 2019

Updated with extra check.

@lburgazzoli lburgazzoli merged commit ebda259 into apache:master Feb 13, 2019
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.

2 participants