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

Add NLUUG Spring 2018 conference to menu.json. #60

Closed
wants to merge 6 commits into from

Conversation

josvos
Copy link

@josvos josvos commented May 3, 2018

No description provided.

@josvos
Copy link
Author

josvos commented May 3, 2018

Sorry for the many versions :-), local testing was not working due to some missing Python modules.

},
{
"name": "Room 3",
"latlon": [52.060788, 5.108085]
Copy link
Owner

Choose a reason for hiding this comment

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

If all three have the same latlon, that's not very useful.. Better to omit the rooms section altogether?

Copy link
Author

Choose a reason for hiding this comment

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

That's fine for me. I just added the 'latlon'' keys because the schema required so and only afterwards I saw the rooms were left out previous year. I assume the schedule per room stays available (as they are listed in the XML anyway)?

Copy link
Owner

Choose a reason for hiding this comment

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

latlon is currently required because it's the purpose of the otherwise optional room section.

So it's better to just omit the room section entirely. I'm pretty sure the schema should allow that, if not that's a bug.

Copy link
Author

Choose a reason for hiding this comment

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

Do you expect me to do something now or can you add the entry without the rooms now?

Copy link
Author

Choose a reason for hiding this comment

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

The conference is next Tuesday. Will the entry be included in time?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah I was waiting for you to remove the room section.... For example https://github.com/Wilm0r/giggity/pull/58/files doesn't have it and is passing the presubmit/CI just fine.

Or do you want to include them as a general venue location pointer? If so, you can just include one room with regex ".*". But I'm not a fan of that idea either..

Copy link
Owner

@Wilm0r Wilm0r left a comment

Choose a reason for hiding this comment

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

I'm holidaying and rather offline, sorry.

},
{
"name": "Room 3",
"latlon": [52.060788, 5.108085]
Copy link
Owner

Choose a reason for hiding this comment

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

Yeah I was waiting for you to remove the room section.... For example https://github.com/Wilm0r/giggity/pull/58/files doesn't have it and is passing the presubmit/CI just fine.

Or do you want to include them as a general venue location pointer? If so, you can just include one room with regex ".*". But I'm not a fan of that idea either..

@josvos
Copy link
Author

josvos commented May 12, 2018

I think it would have been the best to add the entry with the "rooms" section, despite the redundant lat/lon specification. I did change my version now, but I'm not sure how this works with the github pull request stuff.

Wilm0r added a commit that referenced this pull request May 13, 2018
@Wilm0r
Copy link
Owner

Wilm0r commented May 13, 2018

Well what the flying fuck shithub. First you present me a "nope can't merge this there's a conflict". Then I just do it by hand. And now I get a "Squash and merge" button?

Anyway, merged.

I think it would have been the best to add the entry with the "rooms" section, despite the redundant lat/lon specification.

But what would be the point? It's misleading UI if all rooms just point at the same geolocation, and if you want a single entry as mentioned you can just use ".*", but I'd rather not..

@Wilm0r Wilm0r closed this May 13, 2018
aeris pushed a commit to aeris/giggity that referenced this pull request Jun 26, 2018
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

2 participants