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

isExpectedFolder(): log level now generic with default being WARNING #1118

Closed
wants to merge 5 commits into from

Conversation

ssmid
Copy link
Contributor

@ssmid ssmid commented Mar 10, 2025

Fixes #818

@CLAassistant
Copy link

CLAassistant commented Mar 10, 2025

CLA assistant check
All committers have signed the CLA.

@ssmid ssmid requested review from hohwille and julia-cap March 10, 2025 12:02
@jan-vcapgemini jan-vcapgemini added the enhancement New feature or request label Mar 10, 2025
@coveralls
Copy link
Collaborator

coveralls commented Mar 10, 2025

Pull Request Test Coverage Report for Build 13859769305

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 141 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.002%) to 67.659%

Files with Coverage Reduction New Missed Lines %
com/devonfw/tools/ide/tool/ide/IdeToolCommandlet.java 5 77.59%
com/devonfw/tools/ide/io/FileAccessImpl.java 136 68.43%
Totals Coverage Status
Change from base Build 13859584846: 0.002%
Covered Lines: 7826
Relevant Lines: 11139

💛 - Coveralls

@julia-cap
Copy link
Contributor

The changes look good to me.

  • I tested the PR locally in a new project workspace by using the github cli gh pr checkout 1118. After running ide intellij no error message appeared
  • All tests passed locally
  • Maven build runs successfully
    Review done without any further comments.

@julia-cap
Copy link
Contributor

@ssmid Andre, please update your feature branch. I do not have permission to do so for your PR, it seems.

@ssmid ssmid assigned hohwille and unassigned ssmid and julia-cap Mar 13, 2025
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@ssmid thanks for your PR. You even followed the trace from my comment and tried to implement this properly. 👍
However, I have to crush the party here.
I was confused why the error message should be fixed by this PR as this did not make sense to me.
Then I tried to reproduce the issue and figured out that it cannot be reproduced any-more with or without this PR.
I tried to find an explanation for this and found it in our git history...

This is the "old" code that was previously causing the problem:

if (!fileAccess.isExpectedFolder(setupFolder) && !fileAccess.isExpectedFolder(updateFolder)) {

However, the bug #818 was already fixed some time ago but most probably as a side-effect of some other bugfix or feature (most probably #982).
So if there is no bug we also do not need a bugfix.
I hope not to frustrate anybody involved here and hope you take it as a learning to always first reproduce the issue before fixing it.

@hohwille hohwille closed this Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

"Expected folder was not found at D:\projects\IDEasy\settings\intellij\workspace\setup" should be DEBUG
6 participants