Update nodeinfo #446

Merged

zcdunn merged 4 commits from update_nodeinfo_433 into master

zcdunn commented (Migrated from github.com)

I added the repo link to Cargo.toml so that software.repository could be configurable like @rhaamo suggested. I don't know if it's ok to include software.repository without bumping the schema version, but I didn't know if that would break any clients that parse nodeinfo with a hardcoded schema version.

Fix #433 I added the repo link to Cargo.toml so that `software.repository` could be configurable like @rhaamo suggested. I don't know if it's ok to include `software.repository` without bumping the schema version, but I didn't know if that would break any clients that parse nodeinfo with a hardcoded schema version.

❤️ 1

rhaamo commented (Migrated from github.com)

software.repository is nodeinfo 2.1 only, you can however have 2.0 and 2.1 side by side, only the json for 2.1 will returns the repository key.

But that will introduce another thing to do, plume use https://xxx/nodeinfo so it would need to became https://xxx/nodeinfo/version or something like that.

keeping 2.0 would be good anyway for compatibility.

and for the schema version, if you add repository in a 2.0 version it will break on any parser forcing a validation, since 2.0 schema doesn't allow repository.

`software.repository` is nodeinfo 2.1 only, you can however have 2.0 and 2.1 side by side, only the json for 2.1 will returns the repository key. But that will introduce another thing to do, plume use `https://xxx/nodeinfo` so it would need to became `https://xxx/nodeinfo/version` or something like that. keeping 2.0 would be good anyway for compatibility. and for the schema version, if you add repository in a 2.0 version it will break on any parser forcing a validation, since 2.0 schema doesn't allow repository.

zcdunn commented (Migrated from github.com)

Ok. I'll update that. Is it ok to make it two separate endpoints, /nodeinfo/2.0 and /nodeinfo/2.1 since those are the only ones we provide or should the version be parameterized?

Ok. I'll update that. Is it ok to make it two separate endpoints, `/nodeinfo/2.0` and `/nodeinfo/2.1` since those are the only ones we provide or should the version be parameterized?

codecov[bot] commented (Migrated from github.com)

Codecov Report

Merging #446 into master will decrease coverage by 0.12%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
- Coverage   25.58%   25.45%   -0.13%     
==========================================
  Files          63       63              
  Lines        6254     6285      +31     
==========================================
  Hits         1600     1600              
- Misses       4654     4685      +31

# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/446?src=pr&el=h1) Report > Merging [#446](https://codecov.io/gh/Plume-org/Plume/pull/446?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/64d1944715a3fbcb5244f0b94d16eae3b0dd7d7e?src=pr&el=desc) will **decrease** coverage by `0.12%`. > The diff coverage is `0%`. ```diff @@ Coverage Diff @@ ## master #446 +/- ## ========================================== - Coverage 25.58% 25.45% -0.13% ========================================== Files 63 63 Lines 6254 6285 +31 ========================================== Hits 1600 1600 - Misses 4654 4685 +31 ```

elegaanz commented (Migrated from github.com)

Thank you! For the versions, adding a parameter in the route could indeed do the job. And I'm not sure the json! macro allows it, but if possible only add the repository field if the requested version is 2.1

Thank you! For the versions, adding a parameter in the route could indeed do the job. And I'm not sure the `json!` macro allows it, but if possible only add the repository field if the requested version is 2.1

elegaanz (Migrated from github.com) approved these changes

elegaanz (Migrated from github.com) left a comment

Thank you! 👍

Thank you! :+1:

Reviewers

The pull request has been merged as 7bac70a483.

You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.

git checkout -b update_nodeinfo_433 master

git pull origin update_nodeinfo_433

Step 2:

Merge the changes and update on Forgejo.

git checkout master

git merge --no-ff update_nodeinfo_433

git push origin master

No reviewers

No Milestone

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Loading…

There is no content yet.

Labels Milestones

Update nodeinfo #446

Codecov Report

Reviewers

Step 1:

Step 2: