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

Improve documentation #305

Conversation

m-linner-ericsson
Copy link
Member

Applicable Issues

At the Eiffel meetup in Lund during the spring of 2022 we discussed how to improve the documentation. This PR tries to capture a part of that outcome.

Description of the Change

Administrative changes

  • Add table of contents to the intro page
  • Move introduction to a sub-bullet to make it visible

Content additions

  • Add more considerations for custom data
  • Add benefits and implications on the design guidelines

Alternate Designs

I did consider creating a primer.md document but realized that it requires much too much work for adding this content.

Benefits

With more documentation readers have a easier time to understand the concepts the documents tries to convey.

Possible Drawbacks

None that I can think of

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Mattias Linnér mattias.linner@ericsson.com

- Move introduction to a sub-bullet to make it visible
- Add more conciderations for custom data
- Add benefits and implications on the design guidelines
- Add table of contents to the intro page
@m-linner-ericsson m-linner-ericsson requested a review from a team as a code owner August 3, 2022 15:16
@@ -27,5 +27,19 @@ While __data.customData__ affords users extensive freedom in including custom co
* Are there existing Eiffel events and/or event members able to express the information? Using the standard vocabulary and syntax should always be the first option.
* If your use case lacks support in the standard Eiffel vocabulary, there's a chance this is actually a general use case which deserves such support. Create an [Issue](https://github.com/Ericsson/eiffel/issues) about it! It is always better to design a common solution than to implement multiple local adaptations.
* Users defining __data.customData__ members are responsible for them and any compatibility issues. Special considerations or support from standard Eiffel events or syntax can not be expected, unless the custom syntax is proposed to and accepted into the standard Eiffel vocabulary (and consequently is no longer custom).
* Do you introducing custom data to aggregate data on the producer side? If the

Choose a reason for hiding this comment

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

Should this be about not adding data that needs specific contextual knowledge? Or should it be about aggregation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Here I was thinking about aggregation. Do you have any good ideas for improvement?

Choose a reason for hiding this comment

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

"Do you aggregate data on the producer side and add that data as custom data? If the producer aggregates the data it will do so for a specific consumer but we want to create a protocol that can serve any consumer for use cases that has not yet seen the light. Therefore try to use the existing fields and aggregate data on the consumer side."

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 6d5312c

@m-linner-ericsson m-linner-ericsson requested a review from a team August 10, 2022 08:44
customization/custom-data.md Outdated Show resolved Hide resolved
customization/custom-data.md Outdated Show resolved Hide resolved
eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
Comment on lines 51 to 53
* __Coordination__: The Eiffel protocol minimizes coordination effort between
producer and consumer by giving a well-defined specification by defining a
contract between the producer and the consumer.
Copy link
Member

Choose a reason for hiding this comment

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

This sentence doesn't read too well (two subclauses beginning with "by XXXing"). Something like this?

The Eiffel protocol minimizes coordination effort between producers and consumers by defining a specification that the parties can use as a contract.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, reworded it based on you suggestion

Copy link
Member

Choose a reason for hiding this comment

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

Better, but there are a couple of remaining nits. Suggestion:

The Eiffel protocol minimizes coordination efforts between producers and consumers by providing a well-defined specification that the parties can use as a contract.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 5de3f4b ( and do hope I got it this time)

eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
Copy link
Member Author

@m-linner-ericsson m-linner-ericsson left a comment

Choose a reason for hiding this comment

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

Thanks for your comments @magnusbaeck and @k-hallen-ericsson they are addressed in 2493ec2

customization/custom-data.md Outdated Show resolved Hide resolved
@@ -27,5 +27,19 @@ While __data.customData__ affords users extensive freedom in including custom co
* Are there existing Eiffel events and/or event members able to express the information? Using the standard vocabulary and syntax should always be the first option.
* If your use case lacks support in the standard Eiffel vocabulary, there's a chance this is actually a general use case which deserves such support. Create an [Issue](https://github.com/Ericsson/eiffel/issues) about it! It is always better to design a common solution than to implement multiple local adaptations.
* Users defining __data.customData__ members are responsible for them and any compatibility issues. Special considerations or support from standard Eiffel events or syntax can not be expected, unless the custom syntax is proposed to and accepted into the standard Eiffel vocabulary (and consequently is no longer custom).
* Do you introducing custom data to aggregate data on the producer side? If the
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I was thinking about aggregation. Do you have any good ideas for improvement?

customization/custom-data.md Outdated Show resolved Hide resolved
eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
Comment on lines 51 to 53
* __Coordination__: The Eiffel protocol minimizes coordination effort between
producer and consumer by giving a well-defined specification by defining a
contract between the producer and the consumer.
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, reworded it based on you suggestion

eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
Copy link
Member

@magnusbaeck magnusbaeck left a comment

Choose a reason for hiding this comment

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

Kristofer made a good suggestion and I have a few outstanding comments as well.

gives producer and consumer a common way of understanding and describing the
occurrences in the system by defining a contract between them.

Following the guidlines stated, they adress the following issues:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Following the guidlines stated, they adress the following issues:
Following the guidelines stated, they address the following issues:

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 6d5312c

## Benefits and Implications

By following the design guidelines the Eiffel protocol provides a language that
gives producer and consumer a common way of understanding and describing the
Copy link
Member

Choose a reason for hiding this comment

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

This is probably not a hard rule, but in this case I just think it sounds better with the plural form:

Suggested change
gives producer and consumer a common way of understanding and describing the
gives producers and consumers a common way of understanding and describing the

There are other similar cases, I won't make suggestions for all of them.

Copy link
Member Author

Choose a reason for hiding this comment

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

I hope I got them in 6d5312c

magnusbaeck
magnusbaeck previously approved these changes Oct 20, 2022
Copy link
Member

@magnusbaeck magnusbaeck left a comment

Choose a reason for hiding this comment

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

LGTM, but I commented on a minor language nit that we might as well fix.

eiffel-syntax-and-usage/event-design-guidelines.md Outdated Show resolved Hide resolved
Co-authored-by: Magnus Bäck <magnus@noun.se>
@m-linner-ericsson
Copy link
Member Author

LGTM, but I commented on a minor language nit that we might as well fix.

@magnusbaeck Yes, sorry for missing it,

@m-linner-ericsson m-linner-ericsson merged commit 55163e5 into eiffel-community:master Oct 21, 2022
@m-linner-ericsson m-linner-ericsson deleted the improve_documentation branch October 21, 2022 06:59
llconsult added a commit to llconsult/pmeta-docs that referenced this pull request Oct 26, 2022
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

3 participants