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

Standard logging #232

Merged
merged 6 commits into from
Oct 31, 2023
Merged

Standard logging #232

merged 6 commits into from
Oct 31, 2023

Conversation

ivy-rew
Copy link
Member

@ivy-rew ivy-rew commented Oct 31, 2023

No description provided.

Comment on lines -29 to -30
IOFileFilter filter = new SuffixFileFilter(".log");
Collection<File> logs = DiCore.getGlobalInjector().getInstance(IFileAccess.class).getLogFiles(filter);
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the reason why I'd like to drop that adventurous logging feature.
It uses our internal IFileAccess ... which would force use to maintain a master+release/10 branch.
I see no benefit in this strict IO enforcing code ... rather logging to an extra file should be solved with an optional appender.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2023

Test Results

174 tests  ±0   174 ✔️ ±0   2m 22s ⏱️ -29s
  23 suites ±0       0 💤 ±0 
  23 files   ±0       0 ±0 

Results for commit dea68cc. ± Comparison against base commit 4f182bd.

♻️ This comment has been updated with latest results.

}
}
}

public DocFactoryLogDirectoryRetriever getLogDirectoryRetriever() {
Copy link
Member Author

Choose a reason for hiding this comment

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

In theory we're dropping accessible API. I assume that it is not worth to keep it with an empty implementation.

@ivy-rew ivy-rew force-pushed the standardLogging branch 4 times, most recently from 09f3b2e to ba860ed Compare October 31, 2023 09:44
@ivy-rew ivy-rew requested a review from alexsuter October 31, 2023 14:04
@ivy-rew ivy-rew merged commit 9fff446 into master Oct 31, 2023
3 checks passed
@ivy-rew ivy-rew deleted the standardLogging branch October 31, 2023 15:30
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.

1 participant