PubHash fixes #397
PubHash fixes #397
Conversation
dazza-codes
force-pushed
the
pubhash-to-lib
branch
3 times, most recently
from
620859c to
a4b2cd3
Compare
dazza-codes
force-pushed
the
pubhash-to-lib
branch
3 times, most recently
from
0012162 to
b1cc320
Compare
lib/pub_hash.rb
Outdated
| def cap_authors_to_csl(cap_authors) | ||
| # All the CAP authors are an author if they have no role or they have an 'author' role | ||
| authors = cap_authors.map(&:symbolize_keys) | ||
| authors.select! { |author| author[:role].nil? || author[:role].to_s.casecmp('author').zero? } |
There was a problem hiding this comment.
what is the reason for using casecmp here and not something like author[:role].to_s.downcase.include?('author') if we aren't sure of the exact value/case but want to check for the "author" value?
There was a problem hiding this comment.
same comment applies to other uses of casecmp in this class
There was a problem hiding this comment.
It's a rubocop thing, apparently casecmp is more efficient.
There was a problem hiding this comment.
We discussed and concluded today that we should disable the rubocop casecmp cop and revise this PR to avoid using it.
There was a problem hiding this comment.
PR #420 will disable the rubocop casecmp cop.
lib/pub_hash.rb
Outdated
| @@ -109,65 +109,65 @@ def csl_doc | |||
| } | |||
|
|
|||
| # Access to abstracts may be restricted by license agreements with data providers. | |||
| # cit_data_hash["abstract"] = pub_hash[:abstract] unless pub_hash[:abstract].blank? | |||
There was a problem hiding this comment.
presume all the changes in this commit a15f1f1 are rubocop fixes?
dazza-codes
force-pushed
the
pubhash-to-lib
branch
from
b1cc320 to
d7f9f1e
Compare
|
Replaced use of |
dazza-codes
force-pushed
the
pubhash-to-lib
branch
from
7ee5776 to
10e2055
Compare
- RSpec/DescribedClass - RSpec/EmptyLineAfterSubject - RSpec/EmptyLineAfterFinalLet - RSpec/LeadingSubject - RSpec/NamedSubject - RSpec/NotToNot: Use not_to instead of to_not
dazza-codes
force-pushed
the
pubhash-to-lib
branch
2 times, most recently
from
3a37062 to
e29dc92
Compare
Fix #37
Fix #209
Fix #398
The first commit is a
git mvin 82d99ea but github files-changed view is not showing a detailed diff on the PubHash class, it's just showing the entire file moved. This PR will be clearer to review commit-by-commit.The code-climate issues are not from this PR, but I don't want to mark them "wont-fix". The complexity noted in code-climate existed before this PR. It's picking it up because it seems to see the
lib/pub_hash.rbas a new file and everything that was an issue in the old file is somehow fixed. It's not the purpose of this PR to fix all that complexity, although it has helped a bit by extractingparse_author_names.The code-climate issues are old tech-debt that can be addressed in backlog issues #402, #403, and #404.