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

Suggestions for Publons plugin #8

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

asmecher
Copy link

@asmecher asmecher commented Nov 27, 2018

See the various commit messages. These might need to be ported to other branches too. They're mostly cosmetic, but do fix a possible escaping problem around POST parameters to the API.

@asmecher
Copy link
Author

Some suggestions for further tweaking:

  • Some adaptations will be needed in the master branch in for when OJS 3.2 is released. For example:
  • The master branch now uses Smarty v3, and some of their function names have changed (e.g. for registering output filters)
  • The conventions around naming template files have changed
  • The plugin assumes that English content will be available, but that may not be the case.
  • There might be some quirks in the JSON encoding? I'm not sure about the use of JSON_UNESCAPED_UNICODE.
  • Purely cosmetic, but there's a bit of a mix of space-based and tab-based indents. Use whatever you're comfortable with, obviously; we generally use tab-based indents.
  • There appears to be some dead/unused code, e.g. the getPublonsReviews function.

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