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.

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

Documentation update and cleanup #33

Closed
wants to merge 1 commit into from

Conversation

talexb
Copy link

@talexb talexb commented Sep 4, 2013

This update removes 'year' from the CD table definition in both the SQL
and in the class definition, since it wasn't being used. The directory
MyApp/Schema/ResultSet was also removed since the supplied code never
used it.

The results that the sample code produces have been expanded a little so
that they provide answers to the questions asked by the subroutine
calls.

A new section has been added that briefly discusses the results and
explains the techniques that could be used when writing this type of a
programmatic query.

This update removes 'year' from the CD table definition in both the SQL
and in the class definition, since it wasn't being used. The directory
MyApp/Schema/ResultSet was also removed since the supplied code never
used it.

The results that the sample code produces have been expanded a little so
that they provide answers to the questions asked by the subroutine
calls.

A new section has been added that briefly discusses the results and
explains the techniques that could be used when writing this type of a
programmatic query.
@ribasushi
Copy link
Collaborator

Sorry to disappoint, but this pull request is a no-go. year is used in the new "Example" document https://github.com/dbsrgits/dbix-class/blob/master/lib/DBIx/Class/Manual/QuickStart.pod . In fact it was raised on IRC whether we need two versions of a similar document, to which the original author said "meh" ;)

<ribasushi> daxim: food for thought - this seems like 90% of what you wrote (i.e. lots of doc-repetition) https://github.com/dbsrgits/dbix-class/blob/master/lib/DBIx/Class/Manual/Example.pod
<ribasushi> which *is* also just a hands-on practical tutorial
<ribasushi> and shows verbatim the scripts from examples/Schema
<ribasushi> daxim: did you ever considered we now have two quickstarts in a sense? any plan?
<daxim> IMO that pod is fine except for writing the schema manually
<daxim> merge if two pods are too many
<ribasushi> daxim: so it does not strike you as detrimental having 2 documents pertaining to the same thing?
<ribasushi> daxim: I am asking - I long lost the necessary pov to make such a judgement
<daxim> not much detriment
<ribasushi> daxim: aight then, keeping them as is for the time being

Perhaps a fix for this is more worthwhile if you think otherwise (two pods is confusing). Especially since it seems you did get confused as you removed year without consulting neither the Manuals nor the contents of examples/...

On the removal of the ResultSet class - I am on the fense. The fact that it isn't used now, doesn't mean it is not a good idea to create them anyway. In fact I was going to augment the example with base result/resultset clases since this is something that comes in handy 95% of the time, but usually there is too much code to change to make it work properly at the point you need them :(

In any case - closing this pull request without merging for the time being, as a large portion of it needs to be completely changed.

@ribasushi ribasushi closed this Sep 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants