-
Notifications
You must be signed in to change notification settings - Fork 2
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
Updated Joinpoints.ts generation to integrate with Event System #68
base: staging
Are you sure you want to change the base?
Conversation
import eventListener from "./clava/events/EventListener.js"; | ||
import { Event, EventTime } from "./clava/events/Events.js";\n\n` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LARA framework should not know about the existence of Clava, which uses it. This is a sign that the events code currently in the Clava repository should be moved to the LARA framework repository, into the adequate folder.
I suggest moving moving the files to lara-framework/Lara-JS/src-api/lara/events
and update imports accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After looking at the code in PR#157, it is not immediately possible to move the code the the lara-framework, it would need further refactoring in order to separate the Events interface from the Clava-specific implementation.
Since this is still not a finished feature it should not be
merged into the staging branch.
Only when it ceases to be a WIP, could it be merged.
|
I recommend keeping the branch reference so the work isn't lost, but not merging as to not pollute the project with unfinished code. This is a good start but still has a long way to go. |
Updated Joinpoints.ts generation to integrate with Event System
References