|
From: Mark R. <ma...@la...> - 2018-03-04 04:07:55
|
Great it is now released!
Some comments on things I noticed when reading (Version 1.000, PDF
physical page numbers, not print page numbers). I only read the
introduction and the Java part (I'm biased ;):
Page 9: "About the Author" doesn't say much about the author, add a
small biography?
Page 15: Note on use of identity column in Firebird 3 should mention
that creating the sequence and trigger is not necessary if using a
GENERATED BY DEFAULT AS IDENTITY clause
Page 15: Usage of BLOB SUB_TYPE 1 SEGMENT SIZE 80 is curious: a.
SUB_TYPE 1, better would be SUB_TYPE TEXT (more readable/explicit), and
b. as far as I'm aware SEGMENT SIZE has been deprecated, and doesn't
even do anything except maybe for ESQL-usage, which isn't covered in
this guide. I think it should be replaced with just BLOB SUB_TYPE TEXT
to avoid promoting outdated practices.
Page 22: SP_ADD_INVOICE_LINE generates an id for invoice_line_id, but
there is also a trigger that does this. Removing this would also remove
the need for the grant usage to the stored procedure.
Page 27: Name of the zip (and the database) is rather generic, can we
use a more specific name (eg devguide-example.fdb, and
fb_devguide_db_3_0.zip)?
Page 204: How are we going to handle version updates of dependencies?
For example Jaybird 3.0.0 has already been superseded with version
3.0.3. And current latest jOOQ version is 3.10.5, latest Spring is
5.0.4, (or 4.3.14 for 4.x), etc.
Just updating could break examples, but we may want to add a note that
people should check if they need to use newer versions.
Page 206: It might be helpful to add the jetty-maven-plugin. This allows
people to run the example project without having to deploy it (manually)
to tomcat or another servlet engine.
Just add to the list of plugins:
<plugin>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-maven-plugin</artifactId>
<version>9.4.8.v20171121</version>
</plugin>
Then running the example application is just a matter of using
mvn jetty:run
from the commandline (or executing this goal from within your IDE), and
opening http://localhost:8080/ in your browser.
Page 211: Add callout that people must update the directory-element in
the jOOQ configuration in example.xml
I also believe it is possible for jOOQ to have this configuration in the
pom.xml, this might make the code more self-contained.
Page 213: Odd comments in code with a lot of question marks, maybe
originally Russian lost in conversion?
Page 231: Explicit use of a sequence, while the CUSTOMER table has a
trigger to do this. Also, the current user may not have usage rights on
the sequence.
Page 277: The fbjavaex.zip sources contain Russian comments and the
webpage displayed by the project is a mix of Russian and English, it
would be helpful if this is replaced with English.
The documentation contains sources and screenshots that do have English
where the example project has Russian, so maybe the wrong example zip
was published?
I haven't dug to deep in the actual source code, I might have some more
comments on that later ;)
Mark
--
Mark Rotteveel
|
|
From: Helen B. <he...@ii...> - 2018-03-06 21:00:42
|
Sunday, March 4, 2018, 3:02:20 AM, Mark wrote: > Great it is now released! > Some comments on things I noticed when reading (Version 1.000, PDF > physical page numbers, not print page numbers). I only read the > introduction and the Java part (I'm biased ;): > Page 9: "About the Author" doesn't say much about the author, add a > small biography? Denis has since provided this - will be in 1.001 > Page 15: Note on use of identity column in Firebird 3 should mention > that creating the sequence and trigger is not necessary if using a > GENERATED BY DEFAULT AS IDENTITY clause You can add it if you like - add a DOC ticket in Tracker. > Page 15: Usage of BLOB SUB_TYPE 1 SEGMENT SIZE 80 is curious: a. > SUB_TYPE 1, better would be SUB_TYPE TEXT (more readable/explicit), and > b. as far as I'm aware SEGMENT SIZE has been deprecated, and doesn't > even do anything except maybe for ESQL-usage, which isn't covered in > this guide. I think it should be replaced with just BLOB SUB_TYPE TEXT > to avoid promoting outdated practices. You can add it if you like - add a DOC ticket in Tracker. > Page 22: SP_ADD_INVOICE_LINE generates an id for invoice_line_id, but > there is also a trigger that does this. Removing this would also remove > the need for the grant usage to the stored procedure. Comment from Denis would be welcome. > Page 27: Name of the zip (and the database) is rather generic, can we > use a more specific name (eg devguide-example.fdb, and > fb_devguide_db_3_0.zip)? No, I don't think so. We seem to have some limits on path lengths in the CMS; plus any updates to links means updates in multiple locations. > Page 204: How are we going to handle version updates of dependencies? > For example Jaybird 3.0.0 has already been superseded with version > 3.0.3. And current latest jOOQ version is 3.10.5, latest Spring is > 5.0.4, (or 4.3.14 for 4.x), etc. My take on this is that it would be something for the driver coordinators to keep an eye on. > Just updating could break examples, but we may want to add a note that > people should check if they need to use newer versions. You can add it if you like - add a DOC ticket in Tracker. > Page 206: It might be helpful to add the jetty-maven-plugin. This allows > people to run the example project without having to deploy it (manually) > to tomcat or another servlet engine. Same comment as above. > Just add to the list of plugins: > <plugin> > <groupId>org.eclipse.jetty</groupId> > <artifactId>jetty-maven-plugin</artifactId> > <version>9.4.8.v20171121</version> > </plugin> > Then running the example application is just a matter of using > mvn jetty:run > from the commandline (or executing this goal from within your IDE), and > opening http://localhost:8080/ in your browser. ditto ditto ditto. Perhaps you and Denis need to confer about such things? > Page 211: Add callout that people must update the directory-element in > the jOOQ configuration in example.xml > I also believe it is possible for jOOQ to have this configuration in the > pom.xml, this might make the code more self-contained. ditto ditto ditto > Page 213: Odd comments in code with a lot of question marks, maybe > originally Russian lost in conversion? ? > Page 231: Explicit use of a sequence, while the CUSTOMER table has a > trigger to do this. Also, the current user may not have usage rights on > the sequence. > Page 277: The fbjavaex.zip sources contain Russian comments and the > webpage displayed by the project is a mix of Russian and English, it > would be helpful if this is replaced with English. Denis has fixed this. > The documentation contains sources and screenshots that do have English > where the example project has Russian, so maybe the wrong example zip > was published? Nope. It was waaaay more complicated than that. ;-) BTW, I recommended Tracker tickets for changes, etc., as our notification setup from github doesn't seem to be working. Besides, the tracker tickets are more accessible to anyone who wants to follow changes. Remember, Tracker isn't just for bugs. Notice, too, that there is a Document History chapter. It would be highly useful if you would add a row to that for each batch of changes you do, with a link to the Tracker ticket, where applicable. The points you have made concerning the Java section will be more or less applicable to the other platforms as well. Furthermore, we must hope that others will come along with sections for other platforms. Helen |
|
From: Mark R. <ma...@la...> - 2018-03-07 08:53:19
|
On 6-3-2018 22:00, Helen Borrie wrote:
> Sunday, March 4, 2018, 3:02:20 AM, Mark wrote:
<snip>
> You can add it if you like - add a DOC ticket in Tracker.
I'll see what I can do, it may take a while though. I'm currently having
some health problems that result in low energy and lack of
concentration. I was already happy I was able to read it to the end and
make some comments ;)
>> Page 22: SP_ADD_INVOICE_LINE generates an id for invoice_line_id, but
>> there is also a trigger that does this. Removing this would also remove
>> the need for the grant usage to the stored procedure.
>
> Comment from Denis would be welcome.
>
>> Page 27: Name of the zip (and the database) is rather generic, can we
>> use a more specific name (eg devguide-example.fdb, and
>> fb_devguide_db_3_0.zip)?
>
> No, I don't think so. We seem to have some limits on path lengths in
> the CMS; plus any updates to links means updates in multiple
> locations.
Interesting. If the files are uploaded using SCP I wouldn't expect there
to be such issues. Maybe creating an alias higher up the tree in the
file system might be an option; that would also allow for supporting
both old and new links.
>> Page 204: How are we going to handle version updates of dependencies?
>> For example Jaybird 3.0.0 has already been superseded with version
>> 3.0.3. And current latest jOOQ version is 3.10.5, latest Spring is
>> 5.0.4, (or 4.3.14 for 4.x), etc.
>
> My take on this is that it would be something for the driver
> coordinators to keep an eye on.
Ok.
<snip>
> ditto ditto ditto. Perhaps you and Denis need to confer about such
> things?
Ok
>> Page 211: Add callout that people must update the directory-element in
>> the jOOQ configuration in example.xml
>
>> I also believe it is possible for jOOQ to have this configuration in the
>> pom.xml, this might make the code more self-contained.
>
> ditto ditto ditto
>
>> Page 213: Odd comments in code with a lot of question marks, maybe
>> originally Russian lost in conversion?
>
> ?
Example:
// ?????????? ???????????? ???????????
dataSource.setUrl("jdbc:firebirdsql://localhost:3050/examples");
<snip>
> BTW, I recommended Tracker tickets for changes, etc., as our
> notification setup from github doesn't seem to be working. Besides,
> the tracker tickets are more accessible to anyone who wants to follow
> changes. Remember, Tracker isn't just for bugs.
I hadn't noticed yet that the notifications were broken. I wonder if
those are still after-shocks of the issues after the SourceForge
datacenter migration.
I'll check and re-test the configuration.
> Notice, too, that there is a Document History chapter. It would be
> highly useful if you would add a row to that for each batch of changes
> you do, with a link to the Tracker ticket, where applicable. The
> points you have made concerning the Java section will be more or less
> applicable to the other platforms as well. Furthermore, we must hope
> that others will come along with sections for other platforms.
Ok, thanks for the feedback. I hope I can do something with it soon.
Mark
--
Mark Rotteveel
|
|
From: Mark R. <ma...@la...> - 2018-03-07 10:04:15
|
On 7-3-2018 09:53, Mark Rotteveel wrote: > I hadn't noticed yet that the notifications were broken. I wonder if > those are still after-shocks of the issues after the SourceForge > datacenter migration. > > I'll check and re-test the configuration. Possibly the passwords in mailman were lost during the migration (or at least the admin password of the mailing list didn't work for me), so I have set them back to their previous values. However, GitHub notifications still aren't showing up on the list. I have added myself to the addressee list on GitHub, and the notification is showing up on my own address, and it includes the header that should let the list accept it. It looks like it might be a problem somewhere within SourceForge... Mark -- Mark Rotteveel |
|
From: Helen B. <he...@ii...> - 2018-03-08 04:12:09
|
Mark wrote: > On 7-3-2018 09:53, Mark Rotteveel wrote: >> I hadn't noticed yet that the notifications were broken. I wonder if >> those are still after-shocks of the issues after the SourceForge >> datacenter migration. >> >> I'll check and re-test the configuration. > Possibly the passwords in mailman were lost during the migration (or at > least the admin password of the mailing list didn't work for me), so I > have set them back to their previous values. > However, GitHub notifications still aren't showing up on the list. I > have added myself to the addressee list on GitHub, and the notification > is showing up on my own address, and it includes the header that should > let the list accept it. It looks like it might be a problem somewhere > within SourceForge... I don't know how you set that up so I can't test any theory. One thought that occurred to me at the time of the big commit of the fbdevguide files was that I'm fairly sure the SF mailman server has a size limit on all messages, about 75 KB if I remember rightly. I tried to go into the admin interface to see if an over-size message was waiting for action, but the server itself was down at the time. It is up at the moment, but the only message there waiting for action was some of the usual spam. Your guess about some kind of hitch in the infrastructure move is probably not far out. As it happens, a list of files from the initial commit would have been huge, as there were hundreds of files in the sources for the sample projects, including dozens for Windows themes in the two .NET frameworks and the Java app. The important thing for notifications is to signal when someone has committed a change or addition to one book or another. Anyhoo, don't worry yourself about this until you're feeling better. The users have a super reference book to start experimenting with and nothing show-stopping has turned up so far. Helen |