ref: Split protocol types from client file

[rxlabz]

fix #84

📢 Type of change

• Bugfix
• New feature
• Enhancement
• Refactoring

📜 Description

Split the base file in separated files/classes in a /model /protocol subfolder

💡 Motivation and Context

This new structure gives a better perspective of the plugin organisation

💚 How did you test it?

It passes the existing tests

📝 Checklist

• I reviewed submitted code
• I added tests to verify changes
• All tests passing
• No breaking changes

🔮 Next steps

[bruno-garcia]

It's a good direction, I'd propose we call it protocol instead of model though

[bruno-garcia]

LGTM. @marandaneto ?

[rxlabz]

I will fix the analysis score.

[rxlabz]

@bruno-garcia @marandaneto we are loosing 5 points because the changelog file is not in the package directory, can I move the current changelog.md in /dart ?
We're also losing 10 points due to some linting errors for @visibleForTestingmembers, I try to understand why :)

[bruno-garcia]

@bruno-garcia @marandaneto we are loosing 5 points because the changelog file is not in the package directory, can I move the current changelog.md in /dart ?

That lint makes sense since all files included in the directory get pushed to pub.dev as part of the package. Except those on .gitignore. So right now we're missing a changelog

One alternative is a symlink from both sentry and sentry-flutter to the root.

Or perhaps best is to keep two changelogs, one on each package.
With the single changelog approach (ideal IMHO) we should fix the GH actions checks to verify the right file depending on what changes were made (if diff includes stuff under sentry-flutter than that changelog must have a new entry

[rxlabz]

By "single changelog approach" you mean a changelog for each package ? If so I'm agree.
But wouldn't be simpler to have 2 distinct packages in pub.dev: sentry & sentry_flutter ? and maybe 2 Github repos or submodules ?

[marandaneto]

@rxlabz is there a way to overwrite the package-analysis changelog folder (to the root one only)? 2 changelogs IMO is suboptimal, I see a change happening that touch files in both folders, and we'd need to duplicate the changelog anyway.

[marandaneto]

@rxlabz do those file naming changes cause any breaking changes on the client's app? do they need to fix imports or something else?

[rxlabz]

it has no impact unless you use a package internal path which is bad :)

[marandaneto]

@rxlabz I'd suggest if not yet, that you run the sample again and check that everything is working properly as your description only mention "tests are passing".

[rxlabz]

@marandaneto the 2 examples works well on my side, did you noticed any error ?

[marandaneto]

@marandaneto the 2 examples works well on my side, did you noticed any error ?

no, not really, but you didn't check a few things on the PR template so I was afraid that you didn't run it.
I'd suggest to always review your PR and check I reviewed submitted code for example

[marandaneto]

LGTM.

ps: agreed to lower down the pub points to 85 and throw a task to be done at a later point, we'd need to move changelog around or find other solutions.

[rxlabz]

@marandaneto @bruno-garcia I pushed a new commit to level up the score to 95/100

[rxlabz]

The analyzer score on my machine is 95/100 but only 90 on GitHub 🤔

[marandaneto]

The analyzer score on my machine is 95/100 but only 90 on GitHub 🤔

because this one on GH is for the flutter folder, you are probably changing the dart folder, I guess? so happy to merge.

[bruno-garcia]

LGTM! We merge this then and figure out the 5 pub points later then as @marandaneto suggested. We'll be making many PRs before we publish anything anyway