Skip to content

Comments

#6499 change log level to warning for log message issued in exception…#6502

Merged
kcondon merged 1 commit intoIQSS:developfrom
pkiraly:6499-change-log-level-4-exceptions-limited
Jan 10, 2020
Merged

#6499 change log level to warning for log message issued in exception…#6502
kcondon merged 1 commit intoIQSS:developfrom
pkiraly:6499-change-log-level-4-exceptions-limited

Conversation

@pkiraly
Copy link
Member

@pkiraly pkiraly commented Jan 10, 2020

… handling at DataverseServiceBean.findByAlias()

What this PR does / why we need it: This pull request makes error message generated by DataverseServiceBean.findByAlias() separated in the log from FINE messages.

Which issue(s) this PR closes: #6499 Change logging level from fine to warn for exceptions

Is there a release notes update needed for this change?: Improve logging

…tion handling at DataverseServiceBean.findByAlias()
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

There are some whitespace changes in here but if you check the box to ignore them you can see that @pkiraly is really just bumping up the logging for a single line.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 19.504% when pulling ea62fa2 on pkiraly:6499-change-log-level-4-exceptions-limited into af4c0b0 on IQSS:develop.

@pkiraly
Copy link
Member Author

pkiraly commented Jan 10, 2020

@pdurbin Regarding to the whitespace: those lines were indented with tabs while the majority of the file (and the whole codebase) is indented with spaces. I thought it would be not harmful to change this method to be indented with spaces.

@pdurbin
Copy link
Member

pdurbin commented Jan 10, 2020

@pkiraly please see "as small diff as possible to provide independent value" from @djbrooke at #6331 (comment)

@pkiraly
Copy link
Member Author

pkiraly commented Jan 10, 2020

@pdurbin Do you want to create another pull request?

And another question: if I would like to create some code format change, what should be the unit of a pull request: A code block (like this one)? A method? A file?

@pdurbin
Copy link
Member

pdurbin commented Jan 10, 2020

@pkiraly no, this one is fine. No need for another pull request.

I hate to say it but I'm not especially interested in pull requests that only change tabs to spaces or whatever. I agree with @poikilotherm in #5075 that the coding style is inconsistent. Probably what we need is for a tool to enforce a style. We do not have any such tool, currently. I did add a checkstyle config file in pull request #5106. Maybe we can build on top of it?

@pkiraly
Copy link
Member Author

pkiraly commented Jan 10, 2020

@pdurbin Once I had a pull request based on the output of checkstyle (#5337). It was not request which covered almost all the java files in the repository. It was evidently too much, and it was rejected. I am happy to go over on the whole codebase, and fix issues reported by the Maven codestyle plugin, but we need to clarify what is the reasonable unit for pull requests which fix individual code style problems.

@kcondon kcondon self-assigned this Jan 10, 2020
@kcondon kcondon merged commit 3ceba95 into IQSS:develop Jan 10, 2020
@pdurbin
Copy link
Member

pdurbin commented Jan 10, 2020

@pkiraly honestly, a higher priority for me is fixing documentation issues we started talking about last year. You already fixed a few! Thank you! Some others I had in mind are in this column we talked about: https://github.com/orgs/IQSS/projects/3#column-6560524

I see you're also assigned to a couple issues: https://github.com/IQSS/dataverse/issues/assigned/pkiraly . We can certainly remove your assignment from any issue that you're not planning on addressing any time soon. I'd love to keep you (and all contributors) busy with stuff that will deliver a lot of value to the project!

jJIgNRB8

The inconsistency of code style drives me crazy too but I don't think there's an appetite to work on it in 2020. Deep breath. Maybe go read this "gofmt for Java" thread I started on the dev list for some more thoughts: https://groups.google.com/d/msg/dataverse-dev/y2Jpk3szTf8/rckKmP6-BgAJ

@pdurbin pdurbin modified the milestones: 4.18.1, 4.19 Jan 17, 2020
@pkiraly
Copy link
Member Author

pkiraly commented Jun 24, 2021

This PR was developed by Göttingen eResearch Alliance, Germany, and funded by SSHOC, "Social Sciences and Humanities Open Cloud". SSHOC has received funding from the European Union’s Horizon 2020 project call H2020-INFRAEOSC-04-2018, grant agreement #823782.

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.

4 participants