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

inconsistent names of the entryPoint configuration parameter #1488

Merged
merged 2 commits into from Nov 5, 2021

Conversation

energister
Copy link
Contributor

maven 3.6.3 error:

Unable to parse configuration of mojo io.fabric8:docker-maven-plugin:0.37.0:start for parameter entryPoint: Cannot find 'entryPoint' in class io.fabric8.maven.docker.config.RunImageConfiguration

@energister energister changed the title fix examples fix entryPoint examples Aug 24, 2021
@sonarcloud
Copy link

sonarcloud bot commented Aug 24, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@rohanKanojia
Copy link
Member

I guess this is due to entrypoint field present in both BuildImageConfiguration <build> and RunImageConfiguration <run>. It's named differently at both places:

BuildImageConfiguration:

RunImageConfiguration:

@energister
Copy link
Contributor Author

So should I rename the property in RunImageConfiguration instead of changing documentation to make settings names consistent?

@energister energister changed the title fix entryPoint examples inconsistent names of the entryPoint configuration parameter Aug 29, 2021
@rhuss
Copy link
Collaborator

rhuss commented Sep 1, 2021

+1 for making the naming of the fields consistent in the code and name it entryPoint but also keep the entrypoint but deprecate it (like we did here

@Deprecated
@Parameter
private Boolean nocache;
@Parameter
private Boolean noCache;
for noCache, a similar issue)

For the document I would only use entryPoint though, but keeping entrypoint helps for backwards compatibility. Don't forget to mention it in the changelog, too.

thanks !

maven error:
> Unable to parse configuration of mojo io.fabric8:docker-maven-plugin:0.37.0:start for parameter entryPoint: Cannot find 'entryPoint' in class io.fabric8.maven.docker.config.RunImageConfiguration
@codecov
Copy link

codecov bot commented Nov 5, 2021

Codecov Report

Merging #1488 (0387f4d) into master (d4f1584) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##             master    #1488      +/-   ##
============================================
- Coverage     62.13%   62.12%   -0.01%     
- Complexity     2173     2174       +1     
============================================
  Files           166      166              
  Lines          9554     9559       +5     
  Branches       1441     1442       +1     
============================================
+ Hits           5936     5939       +3     
- Misses         3107     3108       +1     
- Partials        511      512       +1     
Impacted Files Coverage Δ
...ic8/maven/docker/config/RunImageConfiguration.java 90.75% <66.66%> (-0.92%) ⬇️

Right now parameter field for ENTRYPOINT seems to be inconsistent across
BuildImageConfiguration and RunImageConfiguration. Use `entryPoint` as
parameter in all places and deprecate others:

- `@Deprecate` `entrypoint` parameter field
- Use `entryPoint` parameter field

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@sonarcloud
Copy link

sonarcloud bot commented Nov 5, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell D 7 Code Smells

77.8% 77.8% Coverage
0.0% 0.0% Duplication

@rohanKanojia rohanKanojia merged commit d3d3618 into fabric8io:master Nov 5, 2021
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.

None yet

3 participants