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

Change links structure to more simple #20

Closed
wants to merge 1 commit into from
Closed

Change links structure to more simple #20

wants to merge 1 commit into from

Conversation

Andrey9kin
Copy link

Change links structure to the list of the key-value pairs - in this way, link types will be just one enum and implementation of the final structure will be much easier for the end-user point of view

Change links structure to the list of the key-value pairs - in this way, link types will be just one enum and implementation of the final structure will be much easier for the end-user point of view
@d-stahl-ericsson
Copy link
Contributor

Just so we're clear, you're suggesting e.g.

"links": [
{"type": "cause", "target": "uuid1"},
{"type": "cause", "target": "uuid2"},
{"type": "iut", "target": "uuid3"}
]

?

@Andrey9kin
Copy link
Author

Exactly

@Andrey9kin
Copy link
Author

If this change is fine with you then I will update description to include example

@d-stahl-ericsson
Copy link
Contributor

I like it, I think it's cleaner. Let's see if we can get another pair of eyes on it.

We'll have to change all our examples, of course.

@p-backman-ericsson
Copy link
Contributor

I like the simplification. One drawback is that I do not think the Json validation will be able to catch that we have more then one link of a single link type.

This will not be detected as illegal
"links": [
{"type": "context", "target": "uuid1"},
{"type": "context", "target": "uuid2"}
]

@d-stahl-ericsson
Copy link
Contributor

True, but we can't validate the legality of links to any significant extent anyway 😦

(I just realized there's a ton of emojis available. I'm so going to abuse them...)

@d-stahl-ericsson
Copy link
Contributor

We'll go for this. We need to update all examples, obviously. Are you willing to do that, @Andrey9kin, then we can merge in both repos at once.

@Andrey9kin
Copy link
Author

@d-stahl-ericsson I push pulled request to examples repo. Would it make sense to merge those two repos? Why do you want them separate?

@d-stahl-ericsson
Copy link
Contributor

The reason to keep them in individual repositories is to keep different types of information separate. This repository is documentation, intended for human consumers. Eiffel-examples primarily contains sample events in various constellations, which you would typically subscribe to in e.g. CI jobs.

@Andrey9kin
Copy link
Author

@d-stahl-ericsson I just thought that those example samples will be consumed by the readers of the protocol and then it would be good to have them in the same place. Also, it helps to sync changes - you can change all you need in one commit securing consistency

@d-stahl-ericsson
Copy link
Contributor

There are pros and cons. The separation of concerns between the repositories is useful, but keeping track of multiple repositories also adds some overhead. We'll see how it plays out in the long run.

@d-stahl-ericsson
Copy link
Contributor

Going back on topic: We need a larger rewrite of the links documentation. Do you want to update this PR, or should I/we do a new one?

@Andrey9kin
Copy link
Author

@d-stahl-ericsson if you have a possibility then please go ahead and do necessary updates. I'm teaching CD this week in Trondheim university and it takes more or less all my time

@d-stahl-ericsson
Copy link
Contributor

Ok, I created a new pull request, #25. Closing this one.

@Andrey9kin Andrey9kin deleted the patch-3 branch July 11, 2016 10:54
@magnusbaeck magnusbaeck added the protocol All protocol changes label Nov 21, 2022
e-backmark-ericsson pushed a commit to e-backmark-ericsson/eiffel that referenced this pull request Dec 21, 2023
The file maps all paths to the repository maintainers' GitHub team.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol All protocol changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants