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

✨ - Show the path of the depency cycle #56

Merged
merged 1 commit into from
Jul 28, 2023
Merged

Conversation

jfrolich
Copy link
Collaborator

No description provided.

@jfrolich
Copy link
Collaborator Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@jfrolich jfrolich force-pushed the path-dependency-cycle branch 3 times, most recently from c86715b to 053ea35 Compare July 28, 2023 12:58
@jfrolich jfrolich changed the base branch from reverse-deps-to-dependents to master July 28, 2023 15:04
@jfrolich jfrolich force-pushed the path-dependency-cycle branch 3 times, most recently from 08121d1 to 7757dc1 Compare July 28, 2023 15:29
@zth
Copy link
Contributor

zth commented Jul 28, 2023

Maybe this doesn't apply, but could this error be written in the same format that the editor tooling expects? Or does this not end up in the compiler log file?

Reason I ask is because we have an issue with the current setup where dependency cycles aren't reported in the editor.

@jfrolich
Copy link
Collaborator Author

Yes, what are the guidelines for the formatting the editor tools expect?
We also have to think to what module to attribute the cycle (maybe the last in the cycle), so if the cycle is Dep01 -> Dep02 -> NS -> NewNamespace.NS_alias -> Dep01 we can attribute it to NS_alias and write it to that compiler log file.

I'll do it as a follow-up PR because I'd like to merge this

@jfrolich
Copy link
Collaborator Author

jfrolich commented Jul 28, 2023

perhaps it is nicest to call out to the compiler, where we provide the compiler with the dependency that causes a cycle, then the compiler can generate an error message with line numbers etc.

Alternatively, we could scan the source file for the module name, and create the error format with the location in the file.

@jfrolich jfrolich merged commit 1ef3ef2 into master Jul 28, 2023
4 checks passed
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