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.

Sign up for GitHub

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

Jump to bottom

range encoding citation update #219

Closed
wants to merge 1 commit into from
Closed

range encoding citation update #219

wants to merge 1 commit into from

Conversation

dericed
Copy link
Contributor

@dericed dericed commented Jul 2, 2020

No description provided.

@michaelni
Copy link
Member

Iam not sure if adding more URLs is a good idea, these will especially in a official slowly changing document like a RFC possibly become dead.
About the mmark bump, like always i see requiring the most recent software suboptimal

@JeromeMartinez
Copy link
Contributor

I am not sure if adding more URLs is a good idea, these will especially in a official slowly changing document like a RFC possibly become dead.

In my opinion the reference makes the life easier for people looking for the source, and archive.org has a copy. If the link is broken in the future, it does not hurt more than if there is no link.
So I am in favor of adding the URL.

@dericed
Copy link
Contributor Author

dericed commented Jul 2, 2020

Hi @michaelni, note that without this PR the make process will break starting with mmark 2.2.8. The range encoder reference uses a seriesInfo element with a @name attribute but no @value attribute. According to rfc7749 the @value attribute is mandatory. Since starting with mmark 2.2.8 this seriesInfo element will be passed along, then xml2rfc will give an error:

xml2rfc --html --v3 "draft-ietf-cellar-ffv1-15.xml" -o "draft-ietf-cellar-ffv1-15.html"
draft-ietf-cellar-ffv1-15.xml(1904): Error: Element seriesInfo failed to validate attributes, at /rfc/back/references[2]/reference[10]/seriesInfo
draft-ietf-cellar-ffv1-15.xml(3): Error: Invalid document before running preptool.
Unable to complete processing draft-ietf-cellar-ffv1-15.xml
make: *** [draft-ietf-cellar-ffv1-15.html] Error 1

@JeromeMartinez
Copy link
Contributor

nudge about this PR, looks like it is the only one blocking before being able to request a new AD review.

@michaelni
Copy link
Member

ive looked at the link again, this is a commercial site, (which atm doesnt work) but google lists for it "Compression Consulting gives technical expert advice for your specific data compression problem. A first inquiry is free." This link is rejected. The site seems an inappropriate choice as reference

@michaelni
Copy link
Member

about mmark, ive updated it and with just that the first commit build fails:

draft-ietf-cellar-ffv1-16.xml(1922): Error: Element seriesInfo failed to validate attributes, at /rfc/back/references[2]/reference[8]/seriesInfo
draft-ietf-cellar-ffv1-16.xml(3): Error: Invalid document before running preptool.
Unable to complete processing draft-ietf-cellar-ffv1-16.xml
Makefile:24: recipe for target 'draft-ietf-cellar-ffv1-16.html' failed

@JeromeMartinez
Copy link
Contributor

@michaelni the website is working on my side (and archive.org has it too) and it links to a PDF:
http://www.compressconsult.com/rangecoder/rngcod.pdf.gz
Would a link to the PDF acceptable? It wouldn't let spec readers get a source (on the website) code though.
(in any case, IMO a link to a commercial site is still better than no link)

@dericed
Copy link
Contributor Author

dericed commented Jul 13, 2020

Hi @michaelni, that specific error message was addressed for me by updated to mmark 2.2.8 as mmarkdown/mmark@13c52eb fixes this issue of missing seriesInfo in references. Can you run mmark -version to verify the right version.

@michaelni
Copy link
Member

@michaelni the website is working on my side (and archive.org has it too) and it links to a PDF:
http://www.compressconsult.com/rangecoder/rngcod.pdf.gz
Would a link to the PDF acceptable? It wouldn't let spec readers get a source (on the website) code though.
(in any case, IMO a link to a commercial site is still better than no link)

And when the site is reorganized the link would then redirect to the front page of that consulting thing.
Its not appropriate for a RFC to contain a link to this IMO

@michaelni
Copy link
Member

Hi @michaelni, that specific error message was addressed for me by updated to mmark 2.2.8 as mmarkdown/mmark@13c52eb fixes this issue of missing seriesInfo in references. Can you run mmark -version to verify the right version.

i did install 2.2.8 before trying so yes it shows "2.2.8".
That said i think the build system needs some more friendly debug support, like a log file or something that contains more info so no back and forth questions are needed, that said, the thing should check versions and fail with clear error messages

@dericed
Copy link
Contributor Author

dericed commented Jul 19, 2020

@michaelni, in regards to a build system, what do you think of using the same approach that @robUx4 used in the ebml and matroska repositories? There's a bootstrap file to facilitate ensuring dependencies are present and meet a minimum version requirement.

@michaelni
Copy link
Member

@dericed , i think a script identifying missing and outdated packages is a good idea. I also think providing commands to the admin of the computer on how to install new enough packages / versions of the tools makes sense. But the actual execution of the commands to install new tools / packages should be up to the admin. I do not believe in black box scripts which directly install tools. ...

@robUx4
Copy link

robUx4 commented Jul 20, 2020

For python scripts they are installed locally to the user using pip. For mmark (in go) the Linux/mac binary is downloaded from the github repo and decompressed in the local folder. The rest there's just a lost instructing the user to install manually (with all the distros and packaging systems it would be too much work anyway).

@dericed dericed mentioned this pull request Jul 29, 2020
Closed
@dericed dericed mentioned this pull request Aug 11, 2020
Closed
mcr
mcr approved these changes Aug 21, 2020
View reviewed changes
Copy link

@mcr mcr left a comment

Choose a reason for hiding this comment

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

seems fine to me

@dericed
Copy link
Contributor Author

dericed commented Aug 21, 2020

Updated to master and removed to requirement for mmark 2.2.8; however, note that part of the references output will be different in mmark versions before 2.2.8 since those neglect to present the seriesInfo/@value attribute as part of the citation.

@dericed dericed mentioned this pull request Aug 21, 2020
Closed
@michaelni
Copy link
Member

Ive applied the 2.2.8 requirement but the link to a random .com site gives me a stomach ache. Can we please refer to some normal academic / primary litrature if we need a reference.

retokromer
retokromer reviewed Aug 22, 2020
View reviewed changes
rfc_backmatter.md Outdated
<front>
<title>Range encoding: an algorithm for removing redundancy from a digitised message.</title>
<title>Range encoding: An algorithm for removing redundancy from a digitized message</title>
<author initials="G." surname="Nigel" fullname=""/>
<author initials="N." surname="Martin" fullname=""/>
Copy link
Contributor

@retokromer retokromer Aug 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

To my knowledge, “G. Nigel N. Martin” is one person and not two, working then at IBM UK, and the title is in UK English, not US.

Copy link
Contributor

Choose a reason for hiding this comment

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

I remembered well:
Screenshot 2020-08-22 at 06 35 39

@retokromer
Copy link
Contributor

@michaelni To my knowledge, the mentioned report by G.N.N. Martin has not been published elsewhere. At least, I could not find it… (I used it for a class in codec programming back in 2015.) However, the main content has been integrated into some of the following articles he published together with other authors mainly in the early 1980s, but I could not suggest one as a full replacement. (I still use these from time to time for my teachings on IT history.)

@dericed mmark 2.2.9 has been released in the meantime ;-)

@dericed
Copy link
Contributor Author

dericed commented Sep 4, 2020

I rebased to correct the representation of the author's name. Without rough consensus on adding an online citation, I suggest to pass on it and removed the target from the PR. I note that RFC6716 provides a citation of the same work without the online reference. Alternatively, we could cite lmgtfy.

JeromeMartinez
JeromeMartinez approved these changes Sep 4, 2020
View reviewed changes
Copy link
Contributor

@JeromeMartinez JeromeMartinez left a comment

Choose a reason for hiding this comment

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

Still in favor of the online link but fine without it, fixing the author name is the most important.

@michaelni michaelni closed this in b11cc1a Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@retokromer retokromer retokromer left review comments

@JeromeMartinez JeromeMartinez JeromeMartinez approved these changes

@mcr mcr mcr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dericed @michaelni @JeromeMartinez @robUx4 @retokromer @mcr