Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/196
This is quite 'e', but I guess there's no way around it, so LGTM.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/195
Code LGTM (I could comment on the files with only cosmetic changes, but I
wont... -ish :)).
Not sure what I can test. I never used Cassandra, and I don't (currently)
have Java 9
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/194
Yeah, I agree, everything looks good.
Nope, not really. I'll take a look in a moment.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/194
D'oh, of course they're blocking, I'm doing the loop inside an update
script! Let me know if you want me to change it. :-)
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/194
Okay, made an even stupider test, this time doing multiple inserts, from
multiple threads (I think we had problems with that on Excel previously) :)
```java
import java.io.File
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/194
D'oh! Forgot about the multiple inserts.
It's getting a bit late here, but I'll try those out Sunday (family visit
tomorrow).
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/194
Made a (bit stupid) test:
```Java
import org.apache.metamodel.data.DataSet;
import org.apache.metamodel.excel.ExcelDataContext;
import org.apache.metamodel.schema.Column;
import
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/194
Okay, I'll give it a spin.
But first, I'll hunt around for a large Excel file (or make one) :)
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/194
LGTM, but I haven't tested yet. Guess such a big jump should have some kind
of manual test. Anything I should look specifically at trying out?
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/195
Some exploding SLF4J bindings.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/178
Looks good!
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/178
I've been reading this a bit, and can't say that I have much technical
input, as I've never really used Kafka, but it sounds a bit like something we
could try as a kind of experimental module? It's
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/174
Beautiful! Merging
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
Merged
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
Works!
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
LGTM
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
Oh man
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
A bit irrelevant to this PR, but I _love_ getting integration tests into
the Travis build. If we can stagger them a bit, I guess we can do all database
engines that are available on Docker? Which
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
Changes looks really good, but Travis is angry. Will look into it when I
have a chance (might not be today)
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
With "for Mongo3 the integration test is a single line from working", I
meant that the MongoDB integration test aaalmost works when enabled, except
for a sin
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
Played around with it a bit in a scratch file and a Dockerized Mongo.
WHEREs with map keys seems to work quite nicely (though I was quite hampered
from my limited knowledge of Mongo), also without
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
Oh damn, forgot about this. Will check tonight.
I meant "not tested" as in "I didn't do a manual test" :)
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/171
Code LGTM, but not tested (at least by me, @Anand49 did you test).
I won't have a chance to test or merge until tomorrow at the earliest, so
someone else might want to do that.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/168
I guess the asfgit bot is only watching master.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/168
Hmmm... I thought @jakubneubauer had enlisted @kaspersorensen's help for
merging this branch to 4.6 and starting the release vote. Any communication
that I missed?
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/167
Merged
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/167
Code LGTM, but this seem like a change that requires an update to the
integration tests. However, they might be quite broken, so it might not make
much of a difference, but at least we should take
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148367755
--- Diff:
core/src/test/java/org/apache/metamodel/QueryPostprocessDataContextTest.java ---
@@ -60,6 +60,49 @@
private final Table table1
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148367081
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -458,15 +459,18 @@ protected String getDefaultSchemaName() throws
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/165
In the one I looked at (QueryPostProcessessDataContext.java), the actual
changes were around 10-15 lines (a little more with the since-introduced
JavaDoc), out of 175 lines changed total, in a 700
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148356156
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -63,24 +64,29 @@
import org.slf4j.LoggerFactory
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/128
Part of the point is it _should_ always round down when you're using
integers. You're changing the type _and_ the behaviour.
Anyway, it still doesn't fix the base problem with unwanted type
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148126992
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -235,38 +240,28 @@ private boolean isSimpleSelect(SelectClause
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148126961
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -216,8 +222,7 @@ public DataSet executeQuery(final Query query
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148127164
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -385,8 +380,8 @@ protected DataSet materializeTable(final Table
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148127149
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -319,13 +314,13 @@ protected DataSet materializeFromItem(final
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148127296
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -63,24 +64,29 @@
import org.slf4j.LoggerFactory
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148127021
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -235,38 +240,28 @@ private boolean isSimpleSelect(SelectClause
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148126842
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -128,7 +134,7 @@ public DataSet executeQuery(final Query query
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148127042
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -235,38 +240,28 @@ private boolean isSimpleSelect(SelectClause
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148127125
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -319,13 +314,13 @@ protected DataSet materializeFromItem(final
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148126897
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -192,8 +198,8 @@ public DataSet executeQuery(final Query query
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148126979
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -235,38 +240,28 @@ private boolean isSimpleSelect(SelectClause
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148126432
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -63,24 +64,29 @@
import org.slf4j.LoggerFactory
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148123395
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -63,24 +64,29 @@
import org.slf4j.LoggerFactory
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/165#discussion_r148126868
--- Diff:
core/src/main/java/org/apache/metamodel/QueryPostprocessDataContext.java ---
@@ -151,8 +157,8 @@ public DataSet executeQuery(final Query query
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/165
Oh man, I never got back to this. Will take a full look tonight.
Anyway, yeah might be more significant than I thought.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/165
Can't look too much now (on the way to work), but looks good. A bit worried
about the size of something that to me is pretty insignificant, though.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/163
Something like that sounds like a better idea to me, yes.
Suggestions:
1. Keep the format, but reject it for all multi-table Datastores. Might of
course be a little weird, interface
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/156
Ah, that might be.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/163
Anyway, up to you. I still think it's a horrible idea, a small convenience
is no worth it for a major source of bugs in client code... Even bugs that will
show itself at unexpected times (again, try
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/163
Every list IS ordered. That's part of the contract of the List
interface...
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/163
On the way home now, will take a proper look when I'm home.
---
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/156
(Of course, there's a chance that checkstyle fixed the lambda thing since I
checked)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/156
Makes sense!
Though some of them are hard. E.g. indentation because checkstyle doesn't
understand lambda expressions, so you can't get it to agree with the
formatters. So I'd have to vote
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/160#discussion_r136401855
--- Diff:
core/src/main/java/org/apache/metamodel/schema/MutableRelationship.java ---
@@ -134,4 +139,17 @@ private MutableRelationship(List
primaryColumns
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/160#discussion_r135997623
--- Diff:
core/src/main/java/org/apache/metamodel/schema/MutableRelationship.java ---
@@ -134,4 +139,17 @@ private MutableRelationship(List
primaryColumns
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/156
@kaspersorensen Regarding explaining settings, we could just export the
formatting rules from the IDEs, and explain in CONTRIBUTE.md to use those. At
least Eclipse and IntelliJ has that option
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/159
We can do like we did in DataCleaner: Have one whole project commit that
does whole-project reformat. It's still a bit annoying history-wise, but since
it will be clearly marked as a reformatting
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/159
I know, but you shouldn't be doing whole-file reformats at all. Even with
w=1, there's still big reformats.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/159
Not really a readable PR, lots of unnecessary reformatting going on.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/158
Unfocused tests also has the problem that it's often unclear what it
intends to test, it makes it harder to understand what the purpose of the code
it is testing was, making its value as part
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/158
Actually, that's one of the worst kind for me (in general, but especially
in this context), since they assume order, and often lead to unfocused tests
(e.g. testing the whole list instead
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/158
Nope, only the version you're maintaining. We have no update plans until
next major release
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/158
BTW, any chance that we could also add this to a 4.x bugfix release? In
DataCleaner we'll soon make a bugfix release, and moving to 5.x for that seems
like a quite a change
---
If your project
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/158
Whoops, missed the query rewriter tests. Will fix.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
GitHub user LosD opened a pull request:
https://github.com/apache/metamodel/pull/158
METAMODEL-1161: Max rows without offset and order by
This fixes Oracle and SQL Server max rows and offset to work properly in
all viable situations, and to use post processing otherwise
Github user LosD commented on the issue:
https://github.com/apache/metamodel-membrane/pull/6
Oh cool! That's perfect then âºï¸
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user LosD commented on the issue:
https://github.com/apache/metamodel-membrane/pull/6
Docker changes LGTM.
Regarding the Postman tests, I guess they're good in itself, but pretty
useless as integration tests, unless you can find a way to make Travis run them
Github user LosD commented on the issue:
https://github.com/apache/metamodel-membrane/pull/5
LGTM!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/156
I'm a big fan of using CheckStyle (but not that there's apparently no
GitHub integration to be found. :disappointed:), so I'm all for it. Also for
more aggressive rules.
---
If your project
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/156
Travis failure seems unrelated. Cassandra startup timed out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/2#discussion_r130610244
--- Diff: swagger-codegen/pom.xml ---
@@ -0,0 +1,123 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/2#discussion_r130609209
--- Diff: swagger-codegen/pom.xml ---
@@ -0,0 +1,123 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/2#discussion_r130540302
--- Diff: swagger-codegen/pom.xml ---
@@ -0,0 +1,123 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/2#discussion_r130540535
--- Diff: swagger-codegen/pom.xml ---
@@ -0,0 +1,123 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/2#discussion_r130539424
--- Diff: .gitignore ---
@@ -8,3 +8,4 @@ target/
*.ipr
*.iws
.vscode/
+.DS_Store
--- End diff --
OSX... Tsk, tsk
Github user LosD commented on the issue:
https://github.com/apache/metamodel-membrane/pull/1
I say "go for it". Unfortunately, I'm working from home today, and I still
can't commit from here, so I'm afraid you'll have to pull the trigger yourself.
(I better try to
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/153
I don't really agree on "ArrayLists should be used", especially for
single-element lists, as a singletonList does not need a backing array, but
it's not really a blocker for me.
-
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/153#discussion_r130093372
--- Diff: core/src/main/java/org/apache/metamodel/CompositeDataContext.java
---
@@ -190,9 +183,7 @@ public Schema getSchemaByNameInternal(String name
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/153#discussion_r130092580
--- Diff:
core/src/main/java/org/apache/metamodel/data/AbstractRowBuilder.java ---
@@ -68,7 +71,7 @@ public AbstractRowBuilder(Column[] columns
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/153#discussion_r130068661
--- Diff: core/src/main/java/org/apache/metamodel/convert/Converters.java
---
@@ -188,7 +191,7 @@ private static void
autoDetectConvertersInternally
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/153#discussion_r130069887
--- Diff: core/src/main/java/org/apache/metamodel/convert/Converters.java
---
@@ -136,15 +135,19 @@ public static DataContext
addTypeConverters(DataContext
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/153#discussion_r130067260
--- Diff: core/src/main/java/org/apache/metamodel/CompositeDataContext.java
---
@@ -190,9 +183,7 @@ public Schema getSchemaByNameInternal(String name
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/153#discussion_r130068074
--- Diff:
core/src/main/java/org/apache/metamodel/DeleteAndInsertBuilder.java ---
@@ -103,7 +103,10 @@ private Row update(final Row original
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/153#discussion_r130070891
--- Diff:
core/src/main/java/org/apache/metamodel/data/AbstractRowBuilder.java ---
@@ -68,7 +71,7 @@ public AbstractRowBuilder(Column[] columns
Github user LosD commented on the issue:
https://github.com/apache/metamodel-membrane/pull/1
Yes, let's do that, I think I have tools to do that.
But let's maybe wait with merging until we have a SNAPSHOT dependency to
work with. I guess the MM release is not far away anyway
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/1#discussion_r129522375
--- Diff:
core/src/main/java/org/apache/metamodel/membrane/app/CachedDataSourceRegistryWrapper.java
---
@@ -0,0 +1,109 @@
+/**
--- End diff
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/1#discussion_r129260484
--- Diff:
core/src/main/java/org/apache/metamodel/membrane/app/CachedDataSourceRegistryWrapper.java
---
@@ -0,0 +1,109 @@
+/**
+ * Licensed
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/1#discussion_r129259910
--- Diff:
core/src/main/java/org/apache/metamodel/membrane/app/CachedDataSourceRegistryWrapper.java
---
@@ -0,0 +1,109 @@
+/**
--- End diff
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel-membrane/pull/1#discussion_r129259959
--- Diff: core/pom.xml ---
@@ -0,0 +1,104 @@
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-ins
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/148
My 2 cents regarding helpers (without having looked at the actual code):
While any kind of utility or helper class is an anti pattern, if the JoinHelper
centers around the same subject
GitHub user LosD opened a pull request:
https://github.com/apache/metamodel/pull/145
METAMODEL-1141-Parse-RFC4180-compliant-CSV
This makes it possible to parse RFC4180-compliant CSV, where the escape
character is a repeated double quote.
OpenCSVs RFC-4180 parser doesn't
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/142
Haven't checked line-by-line, but approach LGTM, and happy to see standard
classes replacing our own.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/142
Aren't all the `@SuppressWarning("resource")` hiding actual bad code?
I remember looking into our `Closeable` usage before and being horrified.
---
If your project is set up for i
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/140
Hmmm... Maybe it's all line-ending changes, GitHub just only shows the
change for .txt files?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user LosD commented on the issue:
https://github.com/apache/metamodel/pull/140
Code LGTM now, but for some files it seems like Git suddenly considers them
changed (since commit e76cd23), even though I can't see any change, not even
white space changes. Not entirely sure
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/140#discussion_r97941969
--- Diff:
dynamodb/src/main/java/org/apache/metamodel/dynamodb/DynamoDbTableCreationBuilder.java
---
@@ -0,0 +1,111 @@
+/**
+ * Licensed
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/140#discussion_r97908427
--- Diff:
dynamodb/src/main/java/org/apache/metamodel/dynamodb/DynamoDbTableCreationBuilder.java
---
@@ -0,0 +1,111 @@
+/**
+ * Licensed
Github user LosD commented on a diff in the pull request:
https://github.com/apache/metamodel/pull/140#discussion_r97731934
--- Diff:
dynamodb/src/main/java/org/apache/metamodel/dynamodb/DynamoDbTableCreationBuilder.java
---
@@ -0,0 +1,111 @@
+/**
+ * Licensed
1 - 100 of 236 matches
Mail list logo
