Breaking Change: SuccessEvent
to SuccessMessage
Transition
#62
Replies: 9 comments
-
I made the change to align with ErrorMessage. In my mind a lot of code seems to have been cobbled together with no real direction, naming methods especially. i brought up these issues years ago with a PR and nothing came of it. i will open a PR to change the places you have mentioned. the documentation is an entirely different beast. this project, in my opinion, has always suffered from poor documentation. we need to properly document all features, that was my next plan. |
Beta Was this translation helpful? Give feedback.
-
i altered the changelog to include this issue for reference as to why it was changed. ErrorMessage is a message and so is SuccessMessage which is why i changed from SuccessEvent to SuccessMessage |
Beta Was this translation helpful? Give feedback.
-
i am keeping this issue open just in case you have objections to my changes |
Beta Was this translation helpful? Give feedback.
-
this became an issue, again, after latest merge |
Beta Was this translation helpful? Give feedback.
-
Yeah, we will keep the way it is for now, we dont want to introduce breaking changes so early in the lib, the idea is wait for the people to migrate over, when everybody is confident on this repo we will be proposing changes that might have breaking changes 😇 |
Beta Was this translation helpful? Give feedback.
-
makes sense. i understand this now. i was just so excited to be able to make changes and have this moving along again. |
Beta Was this translation helpful? Give feedback.
-
successMessage was an old field ( before output expression actions were introduced) .In a way I am not sure if it is used by community. |
Beta Was this translation helpful? Give feedback.
-
Yeah thanks for your input @abbasc52 We will mark that as an obsolete and explain what to use instead, for example output expressions. I don't think we need a new field for this propose, as you mentioned we have that output expression to cover that functionality 🙂 Marking as obsolete will give the community time to think about it |
Beta Was this translation helpful? Give feedback.
-
removing the field is preferrable to renaming it? personally, at first, i used the field, but as @abbasc52 alluded to, actions are my preferred way to go |
Beta Was this translation helpful? Give feedback.
-
Hey @asulwer,
I noticed a breaking change in your branch related to the transition from
SuccessEvent
toSuccessMessage
. This change affects the following file:Rule.cs
at line 43 in the main repositoryRule.cs
at line 43 in your branchAction Required
Update the CHANGELOG:
SuccessEvent
toSuccessMessage
. (I am not sure why we are making this change, so please provide a brief explanation if possible.)Update Documentation:
Getting-Started.md
index.md
.json
files withinRulesEngine\RulesEngine.UnitTest
that referenceSuccessEvent
are updated accordingly.Next Steps
We need to decide whether to:
Your input and any additional context on why this change is being made would be greatly appreciated.
Beta Was this translation helpful? Give feedback.
All reactions