Skip to content

Merge headers into 'finalResults' - #39

Open
jrmartin wants to merge 1 commit into
developmentfrom
feature/435
Open

Merge headers into 'finalResults'#39
jrmartin wants to merge 1 commit into
developmentfrom
feature/435

Conversation

@jrmartin

Copy link
Copy Markdown
Contributor

No description provided.

@jrmartin
jrmartin requested a review from ddelpiano November 29, 2019 13:33

@filippomc filippomc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry I can't get what's the purpose of this pr. Is there any issue I can look to?

@jrmartin

Copy link
Copy Markdown
Contributor Author

Sorry I can't get what's the purpose of this pr. Is there any issue I can look to?

@filippomc It's related to issue #435 in geppetto-vfb VirtualFlyBrain/geppetto-vfb#435

@filippomc

Copy link
Copy Markdown

Sorry I can't get what's the purpose of this pr. Is there any issue I can look to?

@filippomc It's related to issue #435 in geppetto-vfb VirtualFlyBrain/geppetto-vfb#435

Ohhh I filed that issue :D @tarelli what do you think about having a specific issue in the geppetto repo in cases like this? Parhaps the PR may also be enough, if properly described and put in the project. In that case, I'd name the branch something like feature/vfb/435

boolean first = true;
for(QueryResults result : results.keySet())
{
if(finalResults.getHeader().isEmpty())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@jrmartin why two PRs target the same piece of code? Are the both based on development or there is a sequence here? I foresee conflicts

@tarelli

tarelli commented Jan 27, 2020

Copy link
Copy Markdown
Member

@filippomc it's good to have a separate issue but it needs to be clear what it is as it refers to the platform. The original application bug might (should in general) be domain specific but here it needs to become something we understand in more generic terms. So we need to take the time to write that down and reference the application bug as an example of how it could manifest. cc @jrmartin

@filippomc filippomc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Need to add the issue and reconciliation with pr #40 (they are targeting the same piece of code)

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.

4 participants