Simplify & shorten posixlib netdb code paths #2743
Conversation
| /* This is the Linux layout. FreeBSD & macOS have the same size but | ||
| * swap ai_addr and ai_cannonname. FreeBSD documents this, macOS tells | ||
| * whoppers: it documents the Linux order in man pages and implements | ||
| * the FreeBSD layout. | ||
| * | ||
| * Handled in netdbOps below. | ||
| */ | ||
|
|
||
| /* _Static_assert code in netdb.c checks that Scala Native and operating | ||
| * system structute definitions match "close enough". If you change | ||
| * something here, please make the corresponding changes there. | ||
| */ | ||
|
|
There was a problem hiding this comment.
These should probably be a formal Scaladoc - we do generate docs for these. In the C lib you can see them - https://javadoc.io/doc/org.scala-native/clib_native0.4_2.13/latest/scala/scalanative/libc/fenv$.html and also the next two. Complex has a comment on top.
The links to docs are on the main github page.
There was a problem hiding this comment.
Thank you for the timely review.
Eh? Can you educate me on how it is created?
Sorry but I do not know from Scaladoc.
On a quick look, Scaladoc looks like Huge overhead for something that
is well described, modulo OS differences, on 'man' available on any system.
Camel can carry only so many bales.
I thought I was doing well writing it down at least once.
SN devos & maintainers need to know the gruesome details. The wizardry
in the file means that application devos only need to know the usual "use fooOps to
access the fields".
There was a problem hiding this comment.
I took a quick look at Scaladoc conventions. I am not adverse to someone doing
a PR after this one is merged to add Scaladoc but feel that lack of Scaladoc should
not hold up this PR.
In particular, the level of detail in the comment is appropriate for a reader of the
file, but not a general user of the object. Trying to describe it and the switcheroo
in netdbOps to handle it to the general reader of Scaladoc is far more likely to
engender confusion than understanding.
There was a problem hiding this comment.
As just a note with no fancy stuff would suffice for what you have as a start. At the top of the class, object or just before any type or def you can put a comment.
/** This is netdb that does xxx
*
* Another paragraph saying how great it is
*/
object netdb { ...Scalafmt will format it correctly.
There was a problem hiding this comment.
Trying to be a good kid, I checked the Contributor's Guide and noticed the total
absence of a section on Scaladoc and how to generate it within the Scala Native
build.
Not to give in to a bad attitude, I figured out how to generate Scaladoc within
the Scala Native build. Old news to some, but not evident (sbt posixlib3/doc).
Posixlib was all that I needed, so I stopped before figuring out how to generate
the Scaladoc for all projects.
Then I figured out how to view the local copy of the generated Scaladoc (location
and name of generated html files).
What that showed me is a listing of objects, for each object there were methods and
types. I checked this netdb.scala and a couple of others. The descriptions
could certainly be improved by careful work, but they are, IMO, passable.
So, I believe that minimal Scaladoc comes for 'free' and ticks the
"is Scaladoc present" checkbox.
Adding the items Eric suggests should be considered as desirable, but, again IMO,
should not be a hard requirement. Especially in the light of prior art and the total
lack of information in the Contributor's Guide.
|
All green, with the exception of a known & entirely unrelated failure in one qemu. Probably easier if this is merged after related #2742. Less chance of Thank you. |
|
Just added two sections of Scaladoc, one for each object. The The text can be wordsmithed & improved. Best I can do at this hour I have a private message out to Eric to see if we can do some iterative |
LeeTibbert
force-pushed
the
PR_posix_NetdbMinConvert
branch
from
7425dd7
to
b4ce4ab
Compare
|
Rebased & all but usual multiarch suspects pass CI. |
This PR extends the work done in pending PR #2742.
Code in recently merged PRs allows an additional number of netdb related code paths
to be simplified and shortened. Files which are now unnecessary/redundant are removed.
This should bot improve maintaining correctness and reduce execution time. Network
name lookups are expensive at best and are frequently done, usually when somebody
is waiting. Milliseconds gained are worthwhile, as long as correctness prevails.
The number of Windows include files could possibly be reduced/minimized by
someone who knows Windows and has access to a Windows development
system.