Closed ThinkDigitalSoftware closed 3 years ago
New tests need to be properly written ( I suck at that) and old tests need to be updated since the API has changed since then and it's not based on live results.
Tests are failing in Travis.
Thanks for opening this. I'm hoping to spend some time this weekend reviewing it.
OK, great. I'm adding more to it Edit: Oh, that's right, they're being automatically added
I'm glad you responded :) I wanted to contribute to the original package instead of releasing a new one.
Sorry, I have a 3 year old and a 6 month old so my time has been pretty spent for the last few months. Things are starting to get easier, so I should have some time to review it soon.
Awesome, thanks I definitely understand. Tell me what you think about the new tests. I still need to add some more, because things change quickly when parsing HTML, but the main thing is that they're testing with results from the live site, which I know is frowned upon, but I really need to know if the client is going to work with the current state of the “API”
Yeah, it's a pain to keep fake data in sync with something like this. I was actually considering creating another package that is under our control and then targeting that for the tests. I think this is a situation where hitting live data is useful given the nature of the package under test.
On Wed, Oct 23, 2019, 5:17 PM Jonathan White notifications@github.com wrote:
Awesome, thanks I definitely understand. Tell me what you think about the new tests. I still need to add some more, because things change quickly when parsing HTML, but the main thing is that they're testing with results from the live site, which I know is frowned upon, but I really need to know if the client is going to work with the current state of the “API”
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jimsimon/pub_client/pull/7?email_source=notifications&email_token=AABGFT33CJHZR2VXBHLIID3QQC5M7A5CNFSM4IY4HL7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECC47NQ#issuecomment-545640374, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABGFT6DFKEKAQZZH7MAJZTQQC5M7ANCNFSM4IY4HL7A .
Well for the official pub API, it's not that much of an issue. But since my class is parsing the website, it needs to respond to multiple changes. For example, they just added Verified Publishers, which broke any of the packages that switched over to Verified Publishers. I added better error control so that it returns nulls instead of crashing.
I did some work on the PubClient class as well to update with some of the changes, but most of my work is on the PubHtmlParsingClient
When is this PR going to be live and documented?
If you want a live one, you can check my fork. It's more actively updated. You'll need to use the git repo as the source, since I'm trying not to release a duplicate package on pub
I'm still planning to get to this, I just have a lot on my plate at the moment. I'm going to try to review it tomorrow afternoon. I have a new computer so I need to set up a dart environment again, but that shouldn't take too long.
On Mon, Jan 13, 2020, 12:30 PM Jonathan White notifications@github.com wrote:
If you want a live one, you can check my fork. It's more actively updated
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jimsimon/pub_client/pull/7?email_source=notifications&email_token=AABGFT6XNZEQILN65PO4DW3Q5SQMVA5CNFSM4IY4HL7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIZSVTY#issuecomment-573778639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABGFT3UFHMYOEEPU7YLTCTQ5SQMVANCNFSM4IY4HL7A .
Ok. I'm sure I have more commits to add. This branch looks behind. My latest update was in December
On Mon, Jan 13, 2020, 5:53 PM Jim Simon notifications@github.com wrote:
I'm still planning to get to this, I just have a lot on my plate at the moment. I'm going to try to review it tomorrow afternoon. I have a new computer so I need to set up a dart environment again, but that shouldn't take too long.
On Mon, Jan 13, 2020, 12:30 PM Jonathan White notifications@github.com wrote:
If you want a live one, you can check my fork. It's more actively updated
— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/jimsimon/pub_client/pull/7?email_source=notifications&email_token=AABGFT6XNZEQILN65PO4DW3Q5SQMVA5CNFSM4IY4HL7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIZSVTY#issuecomment-573778639 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AABGFT3UFHMYOEEPU7YLTCTQ5SQMVANCNFSM4IY4HL7A
.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jimsimon/pub_client/pull/7?email_source=notifications&email_token=AFPYO7JP4WORQVG6HE76WRDQ5ULHFA5CNFSM4IY4HL7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEI27FTY#issuecomment-573960911, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFPYO7PLSUWLEL73UFGEXKTQ5ULHFANCNFSM4IY4HL7A .
Do you mind updating this PR or opening a new one if that's easier? My initial concern is the failing tests, but that'll likely take some work to fix and make them useful. That work can be done later if it's too large of an effort.
I believe the failing tests aren't because of me, if they're your original tests. The api has changed since
On Tue, Jan 14, 2020, 1:11 PM Jim Simon notifications@github.com wrote:
Do you mind updating this PR or opening a new one if that's easier? My initial concern is the failing tests, but that'll likely take some work to fix and make them useful.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jimsimon/pub_client/pull/7?email_source=notifications&email_token=AFPYO7LFLDGNQMKDADL2E4TQ5YTBJA5CNFSM4IY4HL7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEI6EQMA#issuecomment-574375984, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFPYO7NEHDWD2IR6TARQVMLQ5YTBJANCNFSM4IY4HL7A .
Ah right, no worries then. I'll fix them myself later.
Hm...odd, I just looked at your branch and it seems like this PR should be auto-updating but it's not.
Ah, nevermind I misread something. I'll review this soon.
Sounds good, thanks
Sorry it took so long to get back to this. I made a first pass at reviewing it and had a couple questions.
Time to merge this ;-P
It will be very interesting to have all of this published and a null safety
release
I haven't done any Dart development in a long time. I mostly gave up on Dart when they stopped supporting modern web features in dart:html. I played with Flutter a bit and like it, but I don't do much native mobile development. I've moved on to Typescript for now.
That being said, I can merge this and cut a release. I just won't personally be spending much time on maintaining it. Pull requests will always be welcome though.
I ended up reverting this PR because so many tests were failing. Instead of continuing to attempt to maintain this project, it might be best if anyone interested takes a look at https://github.com/leoafarias/pub_api_client instead. I'm open to revisiting this and trying to get the tests passing if there's something missing from that package that this package supports.
This bypasses the API since it's missing lots of features, but it doesn't replace it, so all existing functionality is still present. I also updated some things that have been removed in the API.
v3.0.3
v3.1.3
v3.1.4
v3.1.5
v3.2.0 - 3.3.0
Add search for PubHtmlParsingClient
Add filters for flutter, web and all packages in search.
Add sorting for:
Add advanced search options available on pub.dev
v3.3.3
BREAKING CHANGE: Version.version has been renamed to Version.semanticVersion.
v3.3.8
v3.4.0
v3.4.1
v3.4.2