[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Added srfi-214: flexvectors
From: |
Vijay Marupudi |
Subject: |
Re: [PATCH] Added srfi-214: flexvectors |
Date: |
Sun, 13 Nov 2022 15:27:22 -0500 |
Hello,
I've made all the suggested edits. Sorry about the delay, universities
get quite busy in the middle of the semester.
> Please use the ChangeLog style for commit messages (see ‘git log’ for
> examples); as a welcome gift, we can do it for you though. :-)
I've tried to do this, please verify this!
>
> • please leave two spaces after end-of-sentence periods (a convention
> eases navigation with Emacs);
>
> • use @dfn to decorate a term the first time it’s introduced, @code
> for identifiers, etc.;
>
> • limit lines to ~80 columns.
Changed.
> Maybe add a first sentence before this one, like: “The @code{(srfi
> srfi-214)} module implements @dfn{flexvectors}, a data structure for
> mutable vectors with adjustable size.”
Added.
> You can drop @bullet{}.
Removed.
> Add a semicolon at the end of the first two lines, a period for the last
> one. @code{ArrayList}.
Added.
> Nitpick: should be:
>
> Copyright (C) 2022 …
>
Changed.
> Please use a single ‘define-module’ clause with #:use-module and
> #:export lines (this is equivalent but that’s what we conventionally
> use and it makes the interface clearer upfront).
>
> Avoid using R6RS and R7RS modules as they pull in a whole lot of stuff.
> For example, maybe you can use (ice-9 binary-ports) instead of (rnrs io
> ports); maybe you don’t need (scheme …) at all because Guile most likely
> has equivalent things built in.
Changed both to fit the convention.
> Don’t export <flexvector> as it would expose implementation details (the
> record interface).
I removed that export.
> In general we try to add docstrings for all public top-level
> procedures. Bonus points if you can do that. :-)
Ah, this would take way too long for me, so I'll leave this up to
another contributor in the future. Would be a great first contribution!
> The rest looks great to me!
>
> Could you send an updated patch?
I've attached the updated patch.
~ Vijay
0001-Added-srfi-214-flexvectors.patch
Description: Text Data
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH] Added srfi-214: flexvectors,
Vijay Marupudi <=