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

Feat: Automatically create parent directories of portPropertyFile path #1761

Conversation

Willena
Copy link
Contributor

@Willena Willena commented Feb 27, 2024

This adds a new option enabling automatic folder creation for the path specified in portPropertyFile.

@Willena Willena force-pushed the feat/port-file-mapping-parrent-folder-create branch from b739b84 to 8a91d91 Compare February 27, 2024 23:49
Copy link

codecov bot commented Mar 2, 2024

Codecov Report

Merging #1761 (4db1e84) into master (110a8f8) will increase coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1761      +/-   ##
============================================
+ Coverage     65.75%   65.76%   +0.01%     
  Complexity     2295     2295              
============================================
  Files           172      172              
  Lines         10181    10184       +3     
  Branches       1405     1406       +1     
============================================
+ Hits           6695     6698       +3     
  Misses         2934     2934              
  Partials        552      552              
Files Coverage Δ
...va/io/fabric8/maven/docker/access/PortMapping.java 90.53% <100.00%> (+0.17%) ⬆️

@rohanKanojia
Copy link
Member

@Willena : Hello, do you think it's a good idea to always create parent directories by default?

@Willena
Copy link
Contributor Author

Willena commented Mar 2, 2024

My initials thought were to add a new parameter set to "false" by default to avoid changing the current behaviour (do not create paths). This should be (unless I forgot something) the current behaviour in the PR.

That beeing said I would personnaly prefer that the plugin always try to create parent paths (and thus I can remove the option and simplifies the code).

@rohanKanojia
Copy link
Member

@Willena : It's good to be defensive. Your changes look good to me.

However, I get the feeling that exposing a new configuration parameter just to ensure whether parent directory of portPropertyFile exists is a bit overkill.

@Willena Willena force-pushed the feat/port-file-mapping-parrent-folder-create branch from 8a91d91 to b67c433 Compare March 2, 2024 22:48
@Willena Willena changed the title Feat: Add configuration allowing portPropertyFile directories to be created Feat: Automatically create parent directories of portPropertyFile path Mar 2, 2024
@Willena
Copy link
Contributor Author

Willena commented Mar 2, 2024

However, I get the feeling that exposing a new configuration parameter just to ensure whether parent directory of portPropertyFile exists is a bit overkill.

I fully agree with you. I've updated the PR to remove the option. Parent directories are now always created.

@rohanKanojia
Copy link
Member

@Willena : Thanks! Could you please add a line to doc/changelog.md regarding this change?

@Willena Willena force-pushed the feat/port-file-mapping-parrent-folder-create branch from b67c433 to 7266c11 Compare March 6, 2024 21:42
@Willena
Copy link
Contributor Author

Willena commented Mar 6, 2024

Done !

@Willena Willena force-pushed the feat/port-file-mapping-parrent-folder-create branch from 7266c11 to 4db1e84 Compare March 6, 2024 21:43
Copy link

sonarcloud bot commented Mar 7, 2024

@rohanKanojia rohanKanojia merged commit b97009a into fabric8io:master Mar 9, 2024
19 of 20 checks passed
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