Details
Diff Detail
- Repository
- rDSTO Storage manager
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/504/ for more details.
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/505/ for more details.
Very nice output !
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 9 | It can be available also as part of a deposit. | |
| 25 | they can provide metadata about different software artifacts, here we deal with origins, but the authorities aren't specifically related to origins. | |
| 61 | I don't think a loader will be dedicated to fetching metadata, because it is dedicated to fetch code and if it changes functionality, it should be a different tool. | |
| 70 | is there a reason gatherers doesn't have the :term: item? | |
| 100 | I don't like future uses and arbitrary.
| |
| 149 | I think a [ ] and adding a second origin_metadata entry, can clarify. [{
'authority': {'name': ..., 'url': ...},
'tool': {'name': ..., 'version': ...},
'discovery_date': ...,
'metadata': b'...'
},
{
'authority': {'name': ..., 'url': ...},
'tool': {'name': ..., 'version': ...},
'discovery_date': ...,
'metadata': b'...'
}] | |
| 151 | the term deposited is too connected to the deposit and here seems that you talk about all authorities. Did you mean, that a list of the latest origin_metadata entries for a given authority is returned instead of all origin_metadata entries? also, this explanation should come before the example. | |
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 25 | We don't support any object other than origins yet. If you think we should support other objects, I can amend the last part of spec accordingly, but I don't think we need it yet. | |
| 70 | Because there is no "gatherer" entry in the glossary yet. | |
| 151 |
No, it returns all of them, but paginated
It's not an example, it's the format of the output | |
Other comment, specs folder might be a better place to hold the file. we might have other specs in swh-docs:-)
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 25 | ok. change a origin to an origin. | |
| 70 | ack. | |
| 151 |
Is that really useful? to have it all?
This explanation should come before the format output :-) | |
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 151 |
Yes, for the same reason we can get the list of snapshots of an origins.
* shrug * | |
looks great !
I've only noted down minor things to be changed.
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 8–9 | I propose "Typical sources for extrinsic metadata are: the hosting place of a repository, which can offer metadata via its web view or API; external registries like collaborative curation initiatives; and out-of-band information provided at source code archival time." That should address @moranegg concern and it feels pretty clear to (the biased) me. | |
| 11–12 | "Since they are not part of the source code, a dedicated mechanism to fetch and store them is needed." | |
| 25 | s/code hosts/code hosting places/ | |
| 26 | It's not the deposit client that has the metadata, that is just a dumb software component; it's the person doing the deposit who has them. Hence, I suggest to use "deposit submitters" here instead. | |
| 35–45 |
| |
| 50 | Having a non ambiguous name here would be helpful to streamline language. From this text I'm assuming you'd be ok with "metadata fetcher"? (It's OK with me.) Hence, s/Metadata fetching tools/*Metadata fetchers*/ here. | |
| 61 | A loader here is consistent with what we discussed f2f though, at least IIRC. The idea was that you might have a generic "git loader", and sub-class it (or whatever) into a "gitlab loader", a "github loader", etc. While the most generic one will only load source code artifacts, the host-specific instances will also fetch extrinsic metadata. TL;DR: this seems correct to me. (and is also consistent with the lister example just below) | |
| 66–67 | Echoing my previous comment, the authority here is the deposit submitter. As we don't' have their identity, for the purpose of the authority table we should probably just use "deposit" here. | |
| 69–70 | gatherer v. fetcher starts becoming clumsy. How about "metadata crawler" here? I'm open to other suggestions if that doesn't work… | |
| 85 | what does the "_by" adds here? wouldn't metadata_authority_get be better/clearer? | |
| 92 | "_by" → ditto | |
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 35–45 |
That was a typo
Do you have example URLs for the deposit you want to use for the deposit? | |
| 69–70 | Much better indeed, thanks! | |
| 85 | Indeed. It made sense when (name, url) was not an intrinsic identifier, but it's no longer true. | |
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/517/ for more details.
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/518/ for more details.
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 11 | s/provided/available/ (sorry, this issue come from my suggestion, i know, but it didn't make sense that way :)) | |
| 14 | missing trailing '.' | |
| 26 | "moral entities" is a false friend from french; just use "entities", I guess? | |
| 35–45 |
I personally don't. Maybe @moranegg does? Alternatively, we can just provide a sample deposit URL with '...' where applicable. | |
| 115–116 | given they're (correctly) grouped together in the result dictionary, maybe you want to also group them together here authority_name/authority_url (as a pair) and same thing for fetcher_name/fetcher_version (unless, dunno, this is mapped to an HTTP API somewhere and it's easier to avoid the packing conceptually they are really two things a fetcher and an authority, so it'd make sense to have 2 args instead of 4 | |
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tox/529/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tox/529/console
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 35 | provider isn't authority now? | |
| 35–45 | https://hal.inria.fr/ and https://hal.archives-ouvertes.fr/ | |
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 35–45 | What is the difference between (name=hal, url= https://hal.archives-ouvertes.fr/) and (name=deposit, url= https://hal.archives-ouvertes.fr/)? | |
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/530/ for more details.
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/532/ for more details.
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 35–45 | Here is the paste for a full table example: | |
| docs/extrinsic-metadata-specification.rst | ||
|---|---|---|
| 35–45 | The following idea surfaced during discussion: | |
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/539/ for more details.