Flutter-Bounty-Hunters / dart-rss

A dart package for parsing RSS & Atom feed
MIT License
23 stars 19 forks source link

Update intl and xml #20

Closed Feichtmeier closed 1 year ago

Feichtmeier commented 1 year ago

Hi 👋

This updates intl and xml and updates the code to not use deprecated members

sudame commented 1 year ago

@Feichtmeier Thank you for your contribution. Please resolve some conflicts, which might be caused by my PR #21 .

Feichtmeier commented 1 year ago

Hey :wave: Some weeks passed and we panicked a little bit about this package here since it looked to us that it might be unmaintained thus we created the fork and also published a package to pub.dev so both MusicPod and Anytime Podcastplayer can move on after the dart3 switch :smile_cat:

However I don't think we really need two packages on pub.dev if this here is maintained Right @amugofjava ? :smile:

So I would close this one and then? Make a PR from our fork master branch again?

amugofjava commented 1 year ago

I think it would make more sense @Feichtmeier to combine efforts on one package than maintaining two that are very similar.

@matthew-carroll are you open to collaboration on your fork and combining efforts?

matthew-carroll commented 1 year ago

@amugofjava Sure - as long as the missions are complementary, I'm happy to bring further changes and features into this package. I don't think I have the context on this discussion. Would anyone like to fill me in?

amugofjava commented 1 year ago

Sure, some time ago you and I both commented on issue 18 and were not getting any response from the developer.

The project appeared to be abandoned so back in May, @Feichtmeier forked the project and I joined later on. So far, this has been to bring the intl and xml libraries up to date, improve linting and add additions to the podcast namespace.

Now that this project is being maintained again, it probably makes sense to combine efforts. I would be happy for that to either be submitting PRs in the usual fashion or being given maintainer rights so I can commit directly - whichever works for you best.

I develop a podcast player app and library and use this library to parse podcast RSS feeds.

matthew-carroll commented 1 year ago

@amugofjava that sounds good to me. I'm happy to review PRs. Please tag me on any such PRs so that I can get an email.

I'm not sure what code quality standards were enforced in this project previously. I'll try and get the contributing guide updated when I can. Until then, I'd like for all new classes and public members to have Dart Docs. I'd also like for new contributions to include appropriate tests. Given that this package implements a protocol, the most effective tests are probably deserialization of real RSS documents. In some cases, it might be useful to write some unit tests or component tests of inner structures.

amugofjava commented 1 year ago

@matthew-carroll,

Yes, that all sounds good to me. Improving Dart docs and testing is also on my list of things to work on and improve. I would also like to reduce the number of nullable types, but that might be a longer term goal.

I'll start by putting together a PR that contains the changes in fork @Feichtmeier & I have worked on and ensure it has enough tests & docs to cover it too.

matthew-carroll commented 1 year ago

PS - The smaller each individual PR, the better. It's much easier to review focused changes. I'd rather review a higher number of narrow PRs than big sweeping PRs.

Feichtmeier commented 1 year ago

Thanks guys for the communication 🙇 Closing this heavily outdated branch/PR