-
Notifications
You must be signed in to change notification settings - Fork 12
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
TCR-46 mod-linked-data #83
Merged
maccabeelevine
merged 7 commits into
folio-org:master
from
maccabeelevine:TCR-46-mod-linked-data
Jan 20, 2025
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
d0df09f
Evaluate TCR-46 mod-linked-data
maccabeelevine a8e2381
API documentation is fixed.
maccabeelevine cad95c6
Interface paths are now prefaced with /linked-data/
maccabeelevine 7a3af2a
Link to sonarqube results
maccabeelevine f303304
PC approved the module's functionality
maccabeelevine 3a18b11
The three module dependencies were approved
maccabeelevine 1a44b21
Note approval of new libraries
maccabeelevine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
# Module acceptance criteria template | ||
|
||
## Module Name | ||
mod-linked-data | ||
|
||
## How to use this form | ||
When performing a technical evaluation of a module, create a copy of this document and use the conventions below to indicate the status of each criterion. The evaluation results should be placed in the [module_evaluations](https://github.com/folio-org/tech-council/tree/master/module_evaluations) directory and should conform to the following naming convention: `{JIRA Key}_YYYY-MM-DD-{module name}.MD`, e.g. `TCR-1_2021-11-17-mod-foo.MD`. The date here is used to differentiate between initial and potential re-evaluation(s). It should be the date when the evaluation results file was created. | ||
|
||
* [x] ACCEPTABLE | ||
* [x] ~INAPPLICABLE~ | ||
* [ ] UNACCEPTABLE | ||
* comments on what was evaluated/not evaluated, why a criterion failed | ||
|
||
## [Criteria](https://github.com/folio-org/tech-council/blob/7b10294a5c1c10c7e1a7c5b9f99f04bf07630f06/MODULE_ACCEPTANCE_CRITERIA.MD) | ||
|
||
## Administrative | ||
* [X] Listed by the Product Council on [Functionality Evaluated by the PC](https://wiki.folio.org/display/PC/Functionality+Evaluated+by+the+PC) with a positive evaluation result. | ||
- PC will be reviewing this module on December 12th. | ||
|
||
## Shared/Common | ||
* [X] Uses Apache 2.0 license | ||
* [X] Module build MUST produce a valid module descriptor | ||
* [X] Module descriptor MUST include interface requirements for all consumed APIs | ||
* [X] Third party dependencies use an Apache 2.0 compatible license | ||
* [X] Installation documentation is included | ||
* -_note: read more at https://github.com/folio-org/mod-search/blob/master/README.md_ | ||
* [X] Personal data form is completed, accurate, and provided as `PERSONAL_DATA_DISCLOSURE.md` file | ||
* [X] Sensitive and environment-specific information is not checked into git repository | ||
- Postgres password in docker-compose.yml, but that's standard. | ||
* [X] Module is written in a language and framework from the [officially supported technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) page[^1] | ||
* [X] Module only uses FOLIO interfaces already provided by previously accepted modules _e.g. a UI module cannot be accepted that relies on an interface only provided by a back end module that hasn't been accepted yet_ | ||
- Three new library dependencies, these are now TCR approved. | ||
- lib-linked-data-dictionary | ||
- lib-linked-data-fingerprint | ||
- lib-linked-data-marc4ld | ||
* [X] Module gracefully handles the absence of third party systems or related configuration | ||
- Works without Kafka. Otherwise, there are no third-party configs that the module should reasonably survive being missing. | ||
* [X] Sonarqube hasn't identified any security issues, major code smells or excessive (>3%) duplication (6); and any disabled or intentionally ignored rules/recommendations are reasonably justified. | ||
* See [Rule Customization](https://dev.folio.org/guides/code-analysis/#rule-customization) details. | ||
* Results: https://sonarcloud.io/summary/overall?id=org.folio%3Amod-linked-data&branch=master | ||
* [X] Uses [officially supported](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) build tools[^1] | ||
* [X] Unit tests have 80% coverage or greater, and are based on [officially supported technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies)[^1] | ||
julianladisch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* Currently 90.7%. https://sonarcloud.io/summary/overall?id=org.folio%3Amod-linked-data&branch=master | ||
|
||
## Backend | ||
* [X] Module's repository includes a compliant Module Descriptor | ||
* -_note: read more at https://github.com/folio-org/okapi/blob/master/okapi-core/src/main/raml/ModuleDescriptor.json_ | ||
* [X] Module includes executable implementations of all endpoints in the provides section of the Module Descriptor | ||
* [X] Environment vars are documented in the ModuleDescriptor | ||
* -_note: read more at [https://wiki.folio.org/pages/viewpage.action?pageId=65110683](https://wiki.folio.org/pages/viewpage.action?pageId=65110683)_ | ||
* [X] If a module provides interfaces intended to be consumed by other FOLIO Modules, they must be defined in the Module Descriptor "provides" section, and must conform to FOLIO [interface naming conventions](https://dev.folio.org/guidelines/naming-conventions/#interfaces). | ||
* [X] All API endpoints are documented in OpenAPI. | ||
* [X] All API endpoints protected with appropriate permissions as per the following guidelines and recommendations, e.g. avoid using `*.all` permissions, all necessary module permissions are assigned, etc. | ||
* -_note: read more at https://dev.folio.org/guidelines/naming-conventions/ and https://wiki.folio.org/display/DD/Permission+Set+Guidelines_ | ||
* [X] Module provides reference data (if applicable), e.g. if there is a controlled vocabulary where the module requires at least one value | ||
- No reference data. | ||
* [X] If provided, integration (API) tests must be written in an [officially supported technology](https://wiki.folio.org/display/TC/Officially+Supported+Technologies)[^1] | ||
* -_note: while it's strongly recommended that modules implement integration tests, it's not a requirement_ | ||
* -_note: these tests are defined in https://github.com/folio-org/folio-integration-tests_ | ||
* [X] Data is segregated by tenant at the storage layer | ||
- Yes, but there is some API cruft that a Jira is addressing: https://folio-org.atlassian.net/browse/MODLD-604 | ||
* [X] The module doesn't access data in DB schemas other than its own and public. | ||
* [X] Any dependencies, other than on defined interfaces, are declared in the README.md. | ||
maccabeelevine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* [X] The module responds with a tenant's content based on x-okapi-tenant header | ||
* [X] Standard GET `/admin/health` endpoint returning a 200 response | ||
* -_note: read more at https://wiki.folio.org/display/DD/Back+End+Module+Health+Check+Protocol_ | ||
* [X] High Availability (HA) compliant | ||
* Possible red flags: | ||
* Connection affinity / sticky sessions / etc. are used | ||
* Local container storage is used | ||
* Services are stateful | ||
* [X] Module only uses infrastructure / platform technologies on the [officially supported technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) list.[^1] | ||
* _e.g. PostgreSQL, ElasticSearch, etc._ | ||
|
||
## TCR Process Improvements | ||
[_Please include here any suggestions that you feel might improve the TCR Processes._] | ||
|
||
[^1]: Refer to the [Officially Supported Technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) page for the most recent ACTIVE release. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List of third party dependencies: https://julianladisch.github.io/folio-dependencies/mod-linked-data-1.0.1-SNAPSHOT.html
Can you elaborate how you came to the conclusion that they are compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used one of those npx tools to generate the list of dependencies, and then compared it against the list at https://apache.org/legal/resolved.html, and there were no issues. This was a month ago so I don't remember the specifics of each license used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npx is for JavaScript, not for maven pom.xml.
Can you specify which tool you've used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the catch -- my not noticing that the method we use for UI modules would not apply.
As a different approach, seeing we already use MojoHaus plugins, I added the license-maven-plugin to the pom
Then executed the third-party-report goal which generated an HTML page with the results -- 321 total dependencies. The HTML is verbose (no consolidated list of licenses), so I ran this cludgy one-liner on in the page's dev console to generated a sorted, unique list of the licenses.
The resulting list of 46 "unique" licenses (really a lot fewer, things are spelled differently) is
Comparing this list to https://apache.org/legal/resolved.html, the only questions I see would be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please click on the link I've posted on 2024-12-16.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Julian, that's a great analysis, thank you.