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

Enable fixes/assits to edit other files #187

Closed
wants to merge 2 commits into from

Conversation

matthiaslarisch
Copy link

No description provided.

@docs-page
Copy link

docs-page bot commented Sep 8, 2023

To view this pull requests documentation preview, visit the following URL:

docs.custom-lint.dev/~187

Documentation is deployed and generated using docs.page.

@vercel
Copy link

vercel bot commented Sep 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
custom-lint-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 12, 2023 2:04pm

@CLAassistant
Copy link

CLAassistant commented Sep 8, 2023

CLA assistant check
All committers have signed the CLA.

void Function(analyzer_plugin.FileEditBuilder builder) buildFileEdit,
);
void Function(analyzer_plugin.FileEditBuilder builder) buildFileEdit, {
String? customPath,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd need this on all addXFileEdit, such as addDartFileEdit

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. customPath parameter added to all addXFileEdit methods

@rrousselGit
Copy link
Collaborator

Could you add tests for this?

Also please sign the CLA 🙏 (cf above)

added the customPath parameter to the other "addXFileEdit" methods
@matthiaslarisch
Copy link
Author

I've added the customPath parameter to all addXFileEdit methods but I'm not sure what kind of test do you want.

@rrousselGit
Copy link
Collaborator

We'd need automated tests invoking those methods with the new parameters.

@rrousselGit rrousselGit changed the title Pull Request for: Write to customized file during DartFix #186 Enable fixes/assits to edit other files Sep 18, 2023
@rrousselGit rrousselGit linked an issue Sep 18, 2023 that may be closed by this pull request
@matthiaslarisch
Copy link
Author

I have checked the tests which are available and the changes of this pull request breaks none of them.
As the changes of this pull request will only be effective when the customPath is explicitly set, no one of the current users will be effected, so I think this change is harmless.
I also looked into the current tests but I'm not sure how I can implement sufficient tests.
Because of the mentioned reasons I will not provide any tests.

@laurentschall
Copy link
Contributor

laurentschall commented Oct 5, 2023

I'll try to find the time to add tests. I guess those tests would require to be triggred by the melos command hereafter:

dart_custom_lint> dart run melos exec --dir-exists=test "dart test"

@rrousselGit
Copy link
Collaborator

@matthiaslarisch Not adding tests is not an option.

I can guide you on how to do it, but it's a must. This PR will not be merged without tests

@rrousselGit
Copy link
Collaborator

Here's a test which runs an assist creating a ChangeBuilder.

test('Assist.testRun', () async {
final assist = MyAssist('MyAssist');

You could reuse this principle to make an Assist which specifies a "path" when creating the changeBuilder

@laurentschall
Copy link
Contributor

This pull request was included in the pull request 199 which are the same codes changes but with matching tests.

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.

Write to cutomized file during DartFix
4 participants