-
Notifications
You must be signed in to change notification settings - Fork 229
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
TEIID-4572 #947
TEIID-4572 #947
Conversation
Dear shawkins, |
Yes, you can submit a new pull request that builds upon these changes. |
This is my new pull request |
Can't connect to any repository: https://github.com/shawkins/teiid.git (https://github.com/shawkins/teiid.git: git-receive-pack not permitted) |
Use the command line instructions from above: git checkout -b shawkins-TEIID-4572 master A conflict has been introduced by the header change on master, but that should be easily resolved. |
these conflicts are not even mine and i don't know how to solve them
for example, N1QLUpdateVisitor
…On Thu, Jun 1, 2017 at 9:27 PM, Steven Hawkins ***@***.***> wrote:
Use the command line instructions from above:
git checkout -b shawkins-TEIID-4572 master
git pull https://github.com/shawkins/teiid.git TEIID-4572
A conflict has been introduced by the header change on master, but that
should be easily resolved.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#947 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQ3ukFrCia4jx5A7cfZ4ohly0Nqc380Eks5r_xCbgaJpZM4NLn07>
.
|
I have rebased and squashed this to a single commit. You can start working from master again with this. |
Don't have permission as told you before Can't connect to any repository: https://github.com/shawkins/teiid.git (https://github.com/shawkins/teiid.git: git-receive-pack not permitted) |
You don't need to push to my fork, you can submit a pull request here or there with additional changes. |
Please check this pull request
amr-alaa-ismail#3
…On Mon, Jun 5, 2017 at 5:04 PM, Steven Hawkins ***@***.***> wrote:
these conflicts are not even mine and i don't know how to solve them
I have rebased and squashed this to a single commit. You can start working
from master again with this.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#947 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQ3ukLXCbOBX6c3WJPMgQnHzkMcQzd4wks5sBBj_gaJpZM4NLn07>
.
|
That doesn't look based correctly either as it shows 490 changes. The most relevant commit I see amr-alaa-ismail@c960471 seems out of date with the changes in this pull request as well. Maybe it would be good to focus on what you are using for a test instance - instructions on how to set it up, or provide a docker image. I can then more easily validate the execution changes. |
Please refer to this commit amr-alaa-ismail/teiid@c629d19
<amr-alaa-ismail@c629d19>
|
Any updates ?!
…On Wed, Jun 14, 2017 at 10:21 AM, Amr Alaa ***@***.***> wrote:
Please refer to this commit ***@***.***
<amr-alaa-ismail@c629d19>
|
#964 is not usable. amr-alaa-ismail@c629d19 - looks like you are trying to make your changes consistent with this pull request, but just looking at a single commit it doesn't appear that it's further than this pull request. Maybe it would be good to focus on what you are using for a test instance - instructions on how to set it up, or provide a docker image. I can then more easily validate the execution changes. The SolrQueryExecution changes have no unit tests, and we need to ensure that no regressions have been introduced. |
4eecac9
to
4367e18
Compare
Merged and updated changes addressing concerns with the visitor. The execution changes still need vetted.