Repository navigation
Breaking Change: SuccessEvent to SuccessMessage Transition #6
Description
Hey @asulwer,
I noticed a breaking change in your branch related to the transition from SuccessEvent to SuccessMessage. This change affects the following file:
Action Required
-
Update the CHANGELOG:
- Document the transition from
SuccessEventtoSuccessMessage. (I am not sure why we are making this change, so please provide a brief explanation if possible.)
- Document the transition from
-
Update Documentation:
- The following documentation files need to be updated to reflect this change:
Getting-Started.mdindex.md
- Additionally, ensure that any
.jsonfiles withinRulesEngine\RulesEngine.UnitTestthat referenceSuccessEventare updated accordingly.
- The following documentation files need to be updated to reflect this change:
Next Steps
We need to decide whether to:
- Revert the change to maintain backward compatibility.
- Proceed with the change and update the changelog and documentation accordingly.
Your input and any additional context on why this change is being made would be greatly appreciated.
Activity
asulwer commentedon Jun 28, 2024
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.
asulwer commentedon Jun 28, 2024
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
asulwer commentedon Jun 28, 2024
i am keeping this issue open just in case you have objections to my changes
asulwer commentedon Jun 29, 2024
this became an issue, again, after latest merge
RenanCarlosPereira commentedon Jun 29, 2024
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 😇
asulwer commentedon Jun 29, 2024
makes sense. i understand this now. i was just so excited to be able to make changes and have this moving along again.
abbasc52 commentedon Jul 1, 2024
successMessage was an old field ( before output expression actions were introduced) .In a way I am not sure if it is used by community.
RenanCarlosPereira commentedon Jul 1, 2024
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
asulwer commentedon Jul 4, 2024
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