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

Code generation based type-providers #233

Closed
wants to merge 11 commits into from
Closed

Conversation

cvogt
Copy link
Member

@cvogt cvogt commented Oct 14, 2013

@amirsh's code-gen based type-providers adapted to work with master (14th Oct 2013). This is not meant to be merged but used for cooperatively reviewing the code.

@cvogt @szeiger

@xeno-by
Copy link
Contributor

xeno-by commented Oct 14, 2013

I'd probably call this "text generation" in order to disambiguate with annotation-based generators that might possibly arrive after annotations become part of Scala. What do you think?

@cvogt
Copy link
Member Author

cvogt commented Oct 15, 2013

I have the feeling people think of code generation as generating actual source code files. Scala macros on the other hand clearly don't do that. So far we distinguished code-generation based type-providers from macro-based type providers, which seems clear to me.

* A meta-model for representing name of column, table, schema and catalog
*/
trait Name {
val name: String
Copy link
Member Author

Choose a reason for hiding this comment

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

why not def?

Copy link
Member

Choose a reason for hiding this comment

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

because it will be defined once and doesn't need more computation. Why define it as def?

Copy link
Member Author

Choose a reason for hiding this comment

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

If there is no reason for using a val here, then using def in the interface and val to implement it in the class seems cleaner to me. IntelliJ seems to warn about it even. See http://stackoverflow.com/questions/7846119/is-there-something-wrong-with-an-abstract-value-used-in-trait-in-scala

@xeno-by
Copy link
Contributor

xeno-by commented Oct 15, 2013

Seeing all these trees flying around, I wonder why not use quasiquotes.

@xeno-by
Copy link
Contributor

xeno-by commented Oct 15, 2013

@cvogt Well, the proposed "text generation" term or "textual code generation", if you will, does highlight this distinction and also has an additional benefit of not leaving room for ambiguities.

@amirsh
Copy link
Member

amirsh commented Oct 15, 2013

@xeno-by Because the decision was not to use quasi-quotes, as it was not in the master branch.

@xeno-by
Copy link
Contributor

xeno-by commented Oct 15, 2013

Well now we have reliable ways to get quasiquotes both in 2.10 and in 2.11. Why not refactor?

* Qualified name for a meta-model of database
*/
case class QualifiedName(parts: List[NamePart]) extends Name {
override val name = parts.mkString(".")
Copy link
Member Author

Choose a reason for hiding this comment

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

The toString method is used in Java/Scala for user readable representations. I don't think we should used it with different meaning. I suggest parts.map(_.name).mkString(".") instead.

@mhlr
Copy link

mhlr commented Nov 8, 2013

Is this still on track to make 2.0?

@cvogt
Copy link
Member Author

cvogt commented Nov 8, 2013

Yes! It's not going to be part of M3 but it will be in RC1

// handles foreign keys
val fks = mTable.getImportedKeys.list
val fksGrouped = fks.groupBy(x =>
(QualifiedName(x.pkTable), QualifiedName(x.fkTable))).mapValues(v => {
Copy link
Member Author

Choose a reason for hiding this comment

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

@amirsh why group by pkTable and fkTable not by fkName?

@cvogt
Copy link
Member Author

cvogt commented Nov 26, 2013

closed in favor of #530

@cvogt cvogt closed this Nov 28, 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants