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

DMN15-44 Clarify equivalence of date vs date and time #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

falko
Copy link
Contributor

@falko falko commented Jul 5, 2023

This is an example pull request that illustrates how the changes of the following issue would be expressed in Git: https://issues.omg.org/browse/DMN15-44

Reviewing this change as a diff is far superior that reading the editing instructions in JIRA.

Rendering before: https://github.com/omg-dmn-taskforce/plain-text-spec/blob/main/10-expression-language-FEEL.adoc#type-conversions

Rendering after: https://github.com/omg-dmn-taskforce/plain-text-spec/blob/DMN15-44/10-expression-language-FEEL.adoc#type-conversions

@falko falko requested a review from barmac July 5, 2023 12:23
@falko falko force-pushed the DMN15-44 branch 2 times, most recently from ea23f58 to d17e24c Compare July 5, 2023 15:43
@falko
Copy link
Contributor Author

falko commented Jul 6, 2023

Currently, the diff itself is not rendered. I wonder if that is due to AsciiDoc or due to the file size. When I switch between source diff and rich diff I'm getting this error:

We’re unable to render the document prior to diffing, sorry.
Next steps:

  • try a smaller AsciiDoc file
  • try Markdown instead

@falko falko mentioned this pull request Jul 20, 2023
2 tasks
@rivettp
Copy link

rivettp commented Sep 29, 2023

Maybe GitHub has changed, but I'm able to see both source and rich diffs (which I didn't even know existed until you mentioned them!). I agree it's a great improvement for basic changes, but have the following reservations:

  • does not obviously allow for an explanation of the change e.g. "change the subject of the sentence from "the" to "a" to make it clear there could be many". In many cases it will be obvious, but would be interested in others' opinions
  • not sure how well it would cope with larger scale reorganizations such as moving a chunk of text from one section to another part of the document or an Annex.

However I think use of diffs should cope with 90% of the issues (rough guess) and we could think of conventions for representing the remainder of the changes to supplement the diffs.

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.

2 participants