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

Changed database to catalog in org.jabref.gui.slr and org.jabref.logic.crawler #9989

Merged
merged 7 commits into from
Jun 8, 2023

Conversation

simar437
Copy link
Contributor

@simar437 simar437 commented Jun 8, 2023

In Issue #9951, it was instructed to replace the term "database" with "catalog" in the code files org.jabref.gui.slr and org.jabref.logic.crawler.

Fixes koppor#615

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. @simar437 I hope you are in the same group as @wy8881. Otherwise, it is very bad style to hijack active pull requests. It was sayd two days (!) ago that @wy8881 will continue.

Maybe, you did not realize that in GitHub terms "issue" is a thing describing that something should be fixed and "pull request" is a code contribution addressing an issue.

Nevertheless, basically, your code looks good. However, I think, you did not try it. That would have been the right action to take to really fix the underlying issue. -- The intention of this exercise is to give an insight about Software Engineering. This discipline is not only about chaninging some code, but to use proper engineering principles to improve software.

Start a SLR:

image

Still shows the old terms.

Thus, your contribution and the one of @wy8881 need to be combined somehow.

I merged the branch of @wy8881 into yours and the UI looks good:

image

Nevertheless, there are small changes. I ask you to do it so that you learn how to use git and your IDE.

@@ -6,3 +6,4 @@ springerNatureAPIKey=${springerNatureAPIKey}
astrophysicsDataSystemAPIKey=${astrophysicsDataSystemAPIKey}
ieeeAPIKey=${ieeeAPIKey}
biodiversityHeritageApiKey=${biodiversityHeritageApiKey}
Catalogs=Create property
Copy link
Member

Choose a reason for hiding this comment

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

There was something going wrong here. This line needs to be removed, doesn't it?

CHANGELOG.md Outdated
@@ -61,6 +61,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- In case the library contains empty entries, they are not written to disk. [#8645](https://github.com/JabRef/jabref/issues/8645)
- The formatter `remove_unicode_ligatures` is now called `replace_unicode_ligatures`. [#9890](https://github.com/JabRef/jabref/pull/9890)
- We improved the error message when no terminal was found [#9607](https://github.com/JabRef/jabref/issues/9607)
- We changed database to catalog in `org.jabref.gui.slr` and `org.jabref.logic.crawler` [#9951](https://github.com/JabRef/jabref/pull/9951)
Copy link
Member

Choose a reason for hiding this comment

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

Changelog messages should be for the end user. We don't have a end-user documentation for the SLR functionality yet Nevertheless one could write following:

Suggested change
- We changed database to catalog in `org.jabref.gui.slr` and `org.jabref.logic.crawler` [#9951](https://github.com/JabRef/jabref/pull/9951)
- In the context of the "systematic literature functionality", we changed the name "database" to "catalog" to use a separate term for online catalogs in comparison to SQL databases. [#9951](https://github.com/JabRef/jabref/pull/9951)

@@ -48,7 +48,7 @@ private QueryResult getQueryResult(String searchQuery) {
}

/**
* Queries all Databases on the given searchQuery.
* Queries all Catalogs on the given searchQuery.
Copy link
Member

Choose a reason for hiding this comment

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

Please also fix casing when you are on it.

Suggested change
* Queries all Catalogs on the given searchQuery.
* Queries all catalogs on the given searchQuery.

Comment on lines 3 to 6
import org.jabref.gui.slr.StudyCatalogItem;

/**
* data model for the view {@link org.jabref.gui.slr.StudyDatabaseItem}
* data model for the view {@link StudyCatalogItem}
Copy link
Member

Choose a reason for hiding this comment

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

If possible, do not add imports just for JavaDoc. -- Use the fully qualified name (FQN) for StudyCatalogItem in JavaDoc.

@koppor koppor mentioned this pull request Jun 8, 2023
6 tasks
@koppor koppor added status: changes required Pull requests that are not yet complete SLR labels Jun 8, 2023
@Akshay-Gole
Copy link

Hi @koppor, Yes @simar437 and @wy8881 are in the same team. We will fix your mentioned changes as soon as possible.

@koppor
Copy link
Member

koppor commented Jun 8, 2023

@Akshay-Gole Thank you for the quick clarification. If possible please work on @simar437's branch (https://github.com/simar437/jabref/tree/fix-for-issue-9951) and push changes there. Then, they appear automatically in this pull request. @simar437 Can give all of you access to his repository.

@simar437
Copy link
Contributor Author

simar437 commented Jun 8, 2023

Hi @koppor,

I made the required changes as you mentioned, thanks for your valuable insights in my PR.

@@ -2225,7 +2225,7 @@ This\ entry\ type\ is\ intended\ for\ sources\ such\ as\ web\ sites\ which\ are\
A\ single-volume\ work\ of\ reference\ such\ as\ an\ encyclopedia\ or\ a\ dictionary.=Неделимая работа или ссылка, как энциклопедия или словарь.
A\ technical\ report,\ research\ report,\ or\ white\ paper\ published\ by\ a\ university\ or\ some\ other\ institution.=Технический отчет, исследовательский отчет, или белая книга, выпущенная институтом или другим учреждением.
An\ entry\ set\ is\ a\ group\ of\ entries\ which\ are\ cited\ as\ a\ single\ reference\ and\ listed\ as\ a\ single\ item\ in\ the\ bibliography.=Набор записей представляет собой группу записей, которые приводятся в виде единой ссылки и перечислены в виде одного элемента в библиографии.
Supplemental\ material\ in\ a\ "Book".\ This\ type\ is\ provided\ for\ elements\ such\ as\ prefaces,\ introductions,\ forewords,\ afterwords,\ etc.\ which\ often\ have\ a\ generic\ title\ only.=Дополнительный материал в "Книге" предназначен для таких элементов, как предисловия, введения, послесловия и т.д.
Copy link
Member

Choose a reason for hiding this comment

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

@Siedlerchr just for info - the space is added by some crowdin magic...

image

Copy link
Member

Choose a reason for hiding this comment

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

Yes these checks have to be handled carefully

@koppor koppor added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers status: changes required Pull requests that are not yet complete and removed status: changes required Pull requests that are not yet complete labels Jun 8, 2023
@Siedlerchr Siedlerchr merged commit c3f13b6 into JabRef:main Jun 8, 2023
Siedlerchr added a commit that referenced this pull request Jan 1, 2024
* upstream/main: (68 commits)
  Fix issue 9863 - Change Tab selection order (#9907)
  New Crowdin updates (#9994)
  Enable editing typo with double clicking on field name in custom entry type (#9977)
  Remove "Field name:" from localization - we already have "Field name" (#9991)
  Changed database to catalog in org.jabref.gui.slr and org.jabref.logic.crawler (#9989)
  Use environment variables for hostname detection (#9910)
  Add cleanup activity for URL field (#9970)
  Fix freezing when fetching IBSN and no results are found (#9987)
  Make Group(Node)TreeViewModel more OO (#9978)
  Fix container for group item count still visible if display count is off  (#9980)
  Fix paste of BibTeX data (#9985)
  Bring back SimplifyBoolean* and UnnecessaryParantheses (and refine guide) (#9981)
  Add new menu entry to remove braces from selection, aka unprotect it. (#9968)
  User specific comment (#9727)
  Bump org.openrewrite.recipe:rewrite-recipe-bom from 1.19.3 to 2.0.0 (#9975)
  Fix typos
  Remove non-needed link
  Fix typos
  Enable RemoveJavaDocAuthorTag (#9973)
  Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9974)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SLR status: changes required Pull requests that are not yet complete status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SLR: Rename "database" to "catalog"
5 participants