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

Rearchitecture switch to multi module #927

Conversation

etienne02
Copy link
Contributor

@etienne02 etienne02 commented Mar 29, 2024

Based on #926

The split to api + impl is not perfect (some are still in api while should be in impl), but this works and some can be completed later.

A lot of files affected but it’s just a move for most of them, no content change.

@etienne02 etienne02 mentioned this pull request Mar 29, 2024
@etienne02 etienne02 force-pushed the rearchitecture-switch-to-multi-module branch 2 times, most recently from 9ed25c9 to a28d108 Compare April 1, 2024 22:44
@etienne02 etienne02 marked this pull request as ready for review April 1, 2024 22:45
@etienne02 etienne02 force-pushed the rearchitecture-switch-to-multi-module branch from a28d108 to 7e34329 Compare April 8, 2024 01:31
@SingingBush
Copy link
Member

a refactor of this size will cause significant problems for some ongoing changes in feature branches. If it absolutely needs doing then we should consider how it can be broken down into smaller changes that can be done in stages

The plugin itself is in the implementation but the API define set of
feature usable by other modules. This fix dependency loop that prevent
from uprading intellij platform gradle plugin.
@etienne02 etienne02 force-pushed the rearchitecture-switch-to-multi-module branch from 7e34329 to a3cbb5a Compare April 8, 2024 12:10
@etienne02
Copy link
Contributor Author

a refactor of this size will cause significant problems for some ongoing changes in feature branches. If it absolutely needs doing then we should consider how it can be broken down into smaller changes that can be done in stages

I tried to move only the minimal to APl, but the there is too much interdependency between all classes. Just extracting the class required by Dub module bring me to the current API stuff you have.

To do it incrementally, I can create a dlang/dlang-impl module with only the plugin.xml file (something like intellij-rust plugin did). This would break the circular dependency and solve our immediate problem. Then we can incrementally move files into a proper architecture with more modules.
In the long term, I would like to have a more modular architecture. I have in mind something similar to intellij-community or intellij-scala.

What do you think about it?

@SingingBush
Copy link
Member

What do you think about it?

That seems a lot more reasonable for now. I am in favour of separation. That's the direction it's been heading for a while but it's a long running evolution. Would you be interested in discussing over slack this evening? I can be online in around an hour and be available for a few hours.

@etienne02
Copy link
Contributor Author

What do you think about it?

That seems a lot more reasonable for now. I am in favour of separation. That's the direction it's been heading for a while but it's a long running evolution. Would you be interested in discussing over slack this evening? I can be online in around an hour and be available for a few hours.

yes

@etienne02 etienne02 closed this May 1, 2024
@etienne02 etienne02 deleted the rearchitecture-switch-to-multi-module branch May 3, 2024 13:49
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.

None yet

3 participants