Skip to content

Removed a number of warnings, added copyright etc#266

Merged
simonharrer merged 2 commits into
JabRef:masterfrom
oscargus:warningremoval2
Oct 29, 2015
Merged

Removed a number of warnings, added copyright etc#266
simonharrer merged 2 commits into
JabRef:masterfrom
oscargus:warningremoval2

Conversation

@oscargus

Copy link
Copy Markdown
Contributor

Cleanups based on warnings, primarily:

  • Removed right-hand side arguments of diamond operators
  • Static functions
  • Commented empty blocks
  • Added try-with
  • Avoided hiding variables

Included copyright statements.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should the resort list comment be removed, too? Or is the implementation below some kind of resorting?!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I interpreted the first line as a comment and the second as a comment-out. :-) But reading it, I see your point. Will fix.

@koppor

koppor commented Oct 27, 2015

Copy link
Copy Markdown
Member

Other than my minor comments: 👍

@oscargus

Copy link
Copy Markdown
Contributor Author

When I saw System.*.print* I replaced it with LOGGER, but I haven't searched for it (yet). But, yes, I can changed that.

OK, will try and look for those else if:s.

@oscargus

Copy link
Copy Markdown
Contributor Author

XMPUtil is a command line tool, so I think that it makes sense to keep it as is there?

@koppor

koppor commented Oct 28, 2015

Copy link
Copy Markdown
Member

I don't get the comment about XMPUtil. Isn't it just a plain class? May it also be used as executable? Even if, we should use logging, because we mainly use it in JabRef.

@matthiasgeiger

Copy link
Copy Markdown
Member

@koppor The only System.out.'s are in the main()-method of the class (and in a method which explains the usage() of the CLI mode. However, it seems to be that the possibility of the standalone usage is documented nowhere...

Remove it? Leave it as is? Or use a Logger even for the CLI infos?

@koppor

koppor commented Oct 28, 2015

Copy link
Copy Markdown
Member

Leave as is. Add a comment above the line (for our greppers). Add a note at our ticket to extract as external lib. That will be a standalone tool then! (lib+cmd using that lib).

@simonharrer

Copy link
Copy Markdown
Contributor

The main method (CLI) cannot be called at the moment. I checked 2.10 and we only shipped it without building an additional jar with a different main method. Why not just remove this then?

@koppor

koppor commented Oct 28, 2015

Copy link
Copy Markdown
Member

I believe, it might be a useful tool for some power users.

@matthiasgeiger

Copy link
Copy Markdown
Member

Maybe 😉
But wouldn't it be better to integrate it in the normal JabRef CLI?

@simonharrer

Copy link
Copy Markdown
Contributor

-> dev call :)

@simonharrer

Copy link
Copy Markdown
Contributor

Thank you @oscargus for this PR.

simonharrer added a commit that referenced this pull request Oct 29, 2015
Removed a number of warnings, added copyright etc
@simonharrer simonharrer merged commit bb7c0f2 into JabRef:master Oct 29, 2015
@oscargus oscargus deleted the warningremoval2 branch November 1, 2015 16:44
@koppor koppor mentioned this pull request Apr 27, 2017
koppor pushed a commit that referenced this pull request Oct 1, 2022
cb98d36691 copied .github/workflows/merge.yaml .github/workflows/sheldon.yaml from styles
62dcca65d7 Update locales-nn-NO.xml (#267)
eb8587463f Update locales-nb-NO.xml (#266)
bd502ffb0d Update locales-pt-PT.xml (#262)
2dcc82ced1 Update locales-it-IT.xml (#263)

git-subtree-dir: buildres/csl/csl-locales
git-subtree-split: cb98d366912a0c0be361880e12fbc43cef6d383e
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