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

Adsurl to url #2861

Merged
merged 3 commits into from
May 24, 2017
Merged

Adsurl to url #2861

merged 3 commits into from
May 24, 2017

Conversation

jhshinn
Copy link
Contributor

@jhshinn jhshinn commented May 23, 2017

This moves the adsurl field to the url field. The current ADS fetcher removes the field adsurl. The adsurl field is the only url given by the ADS database to all entry in the database. I think it is better to keep the url information rather than deleting it. It is more profitable when considering the copy command 'Edit - Copy BibTex key and link.' (I have run gradlew check and there was no problem.)

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@Siedlerchr
Copy link
Member

Thank you very much for your contribution. From my side the code looks good. When another dev gives his okay, we can merge it

@koppor
Copy link
Member

koppor commented May 23, 2017 via email

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Just one very small remark. After this is fixed we can merge it. Thanks for you PR!

new FieldFormatterCleanup("adsnote", new ClearFormatter()).cleanup(entry);
new FieldFormatterCleanup("adsurl", new ClearFormatter()).cleanup(entry);
// Move adsurl to url field
new MoveFieldCleanup("adsurl","url").cleanup(entry);
Copy link
Member

Choose a reason for hiding this comment

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

Can you please use FieldName.URL instead of "url". Moreover, add a space after the comma.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tobiasdiez I've revised the file as you commented. Thanks for your comment-

use FieldName.URL instead of "url" as commented by @tobiasdiez
@jhshinn
Copy link
Contributor Author

jhshinn commented May 24, 2017

@koppor Do you mean the stuff under build/test-results/test/ directory? (created after running ./gradlew check)
+++ updated
@koppor I confused the meaning of "Tests". I had thought I had to upload some test results created by running gradlew check. I've modified the checkbox accordingly (Thanks a lot, @lynyus ).

@tobiasdiez tobiasdiez merged commit dbc9125 into JabRef:master May 24, 2017
@jhshinn
Copy link
Contributor Author

jhshinn commented May 26, 2017

I'd like to take this opportunity to thank all the JabRef developers for their excellent jobs. JabRef is getting wonderful and wonderful! The contribution guide is clear and easy to follow as well. I'm a newbie in Java coding and github things, but I didn't have much trouble. I'm happy I was able to make some contribution.

@jhshinn jhshinn deleted the adsurl-to-url branch May 26, 2017 00:57
Siedlerchr added a commit that referenced this pull request May 27, 2017
* upstream/master: (23 commits)
  Implement #2785: resort groups using drag & drop (#2864)
  Add Library of Congress as ID-fetcher (#2865)
  Fix export and import of MS office day/year/month acessed fields (#2862)
  Adsurl to url (#2861)
  Update LICENSE.md
  Update
  Update LICENSE.md
  Update license file so that github recognize it properly
  Improve Issue Template Using a Collapsible Log Area
  Fix #2852: Improve performance of group filtering.
  Rename GroupSelector to GroupSidePane
  Fix #2843: Show information correctly in entry editor
  Remove old entry editor code related to focus selection
  Implement feedback
  Menu Greek Translation (#2836)
  Relaxed the regex to also match negative timezone formats when parsing pdf annotation dates (#2841)
  Update localization
  Remove unnecessary group code (and move remaining settings to preferences)
  Add Local Maven repo as first lookup resource, to avoid having duplicate libs in gradle and maven
  Implement #2786: Allow selection of multiple groups
  ...
Siedlerchr added a commit that referenced this pull request Jun 3, 2017
* upstream/master: (38 commits)
  Add link to "feature branch workflow"
  Support Annotations Created by Foxit (#2878)
  Fixes jacoco by excluding the fetcher tests from analysis (#2877)
  Fix entry editor (#2875)
  update bcprov-jdk15on from 1.56 -> 1.57
  update assertj-swing-junit from 3.5.0 -> 3.6.0
  update mockito-core from 2.7.22 -> 2.8.9
  update jfx from 0.11.0 -> 0.11.1
  update google guava from 21.0 -> 22.0
  Fix Divide by zero exception if rows is zero in Entry Editor Tab (#2873)
  Implement #2785: resort groups using drag & drop (#2864)
  Add Library of Congress as ID-fetcher (#2865)
  Fix export and import of MS office day/year/month acessed fields (#2862)
  Adsurl to url (#2861)
  Update LICENSE.md
  Update
  Update LICENSE.md
  Update license file so that github recognize it properly
  Improve Issue Template Using a Collapsible Log Area
  Fix #2852: Improve performance of group filtering.
  ...
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.

5 participants