-
-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: Start new documentation #663
Conversation
Codecov Report
@@ Coverage Diff @@
## master #663 +/- ##
==========================================
- Coverage 76.71% 76.68% -0.03%
==========================================
Files 195 195
Lines 7008 7008
==========================================
- Hits 5376 5374 -2
- Misses 1632 1634 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Hi @M123-dev!
I currently have other urgent priorities around off-dart and Smoothie, so if we could avoid less important PR like doc I would appreciate that.
My comments:
- Please stick to https://dart.dev/guides/language/effective-dart/documentation, and more specifically please say whatever you have to say in just one line, and if you have more to say skip a line and say it
- Don't say anything that the developer can already see, like the returned type or the default values.
- Please don't put breaking changes in an unrelated doc PR - there are so many lines you added that github doesn't even bother to show the diffs - please put a
TODO
instead, or at least mention it in your PR comment (and in the "how to upgrade" section of theREADME.md
)
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.
Hi @M123-dev!
I have minor comments - feel free to ignore them.
In addition to them, your comment about getOrderedNutrients
is not valid anymore, as you're finally still using cc
(and not a country).
Another comment: as the PR title is going to be replicated to pub.dev, I'm not sure if feat: Start new documentation
is relevant. I would say
refactor:
, but we know it's not a good idea, versioning-wisedocumentation update
@@ -6,6 +6,24 @@ import 'language_helper.dart'; | |||
import 'product_fields.dart'; | |||
|
|||
/// Query Configuration for user-related searches. | |||
/// Get products a user created, photographed, |
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.
By the way this class is now deprecated, so no use to be verbose.
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.
What would be the right way to do this now, what is a normal query
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.
Cf. Smoothie, cf. integration tests.
Using the standard product list query configuration, with adapted parameters.
Co-authored-by: monsieurtanuki <fabrice_fontaine@hotmail.com>
Co-authored-by: monsieurtanuki <fabrice_fontaine@hotmail.com>
Co-authored-by: monsieurtanuki <fabrice_fontaine@hotmail.com>
Thanks for your review @monsieurtanuki, I applied all your suggestions |
What
Started with a new documentation here, the core principles are:
Here is the updated readme rendered