prestodb/sql

Code conventions

manticore-projects opened this issue · 3 comments

manticore-projects commented

Greetings!

Please pardon my ignorance and my German habit of speaking blunt.
I added a lot of tests and got it working but I struggle a lot with the particular infrastructure of the project:

  1. the Maven Airbase Plugin enforces Checkstyle rules with a certain formatting.
    This formatting is unusual for someone used to Sun/Google conventions. I do not even know the name of this particular convention.

If feel like if such a formatting is enforced then at least we needed to state the name and the rules and even better, provide a formatter (e. g. Spotless) for reformatting accordingly.

Can you tell me, what coding convention to follow please? Then I would implement the formatter stuff.

  1. the Airbase Plugin also insists on Maven Enforcer Plugin. This seems to break Netbeans 16 and also conversion to Gradle (I am sorry, I have a very personal issue with Maven in general.) IntelliJ does not work either.

Running it with Maven from the command line works perfectly fine. Nevertheless simple people like me may prefer IDEs.

Would you mind it when I add an optional Gradle Build script?
Right now I spent 80% of my time fighting with Maven and less than 20% in bringing the tests in.

  1. I get behind the separation of the Grammar into text fragments but with my limited understanding I also see a few disadvantages:

a) IntelliJ has actually a very useful JavaCC Grammar plugin which makes browsing and refactoring a Grammar very comfortable. This won't work with the Text Fragments.

b) the concat script works only on Linux/Unix right now (very fine by me, however I see platform independence as a plus)

c) it does not answer yet the question for a real modular, dialect based grammar, where we would overwrite tokens or productions as per dialect. I wonder if a kind of template based grammar would be a feasible approach -- at the cost of running the Grammar through the template engine first (although I would argue that the current cat *.txt is the simplest form of a template engine already)

A completely naive idea: what about having one Standard Compliant Grammer (in JavaCC format) and then sub-folders for each dialect, where we provide Grammar fragments, which will overwrite fragments of the main grammar. Although I understand the challenge of having an additional pre-parsing/mangle step.

Cheers!

manticore-projects commented

Sorry for one more question:

What would you think about moving all Build/Infrastructure related config files for PMD, CheckStyle, Formatter etc. into a config folder outside of the src folder?

kaikalur commented

We use the main prestodb/preso style convention. But I don't think we need to follow that (though I prefer that). Feel free to make it anything you want.

Gradle (I am sorry, I have a very personal issue with Maven in general.) IntelliJ does not work either.

Me neither lol - but again we were following mostly the presto project setup here.

Would you mind it when I add an optional Gradle Build script?

Sure! Just add some documentation on how to use it.

a) IntelliJ has actually a very useful JavaCC Grammar plugin which makes browsing and refactoring a Grammar very comfortable. This won't work with the Text Fragments.

For now, we can simply add the generated complete grammar to the repo as well. We don't have grammar override concept in javacc so this is the easiest way to split. Feel free to fix up the concat options.

manticore-projects commented

Updates:

  1. Netbeans 16 still fails, I sent a support request to the Netbeans List.
    Maven from the Terminal Console works fine and I also got it working on IntelliJ (good for testing the SQLs).

  2. I made friendship with the Airlift conventions and supplied an Eclipse Formatter Configuration, which can be applied via mvn spotless:apply -- no issue since.

  3. I have provided a minimal Gradle Build for the Website only. I will amend this eventually in order to provide full build and test capabilities.

  4. On the Grammar itself, I found so many basic SQL features failing that Dialects and Modularity are not a concern right now. First we will need to make the standard compliant Statements work.