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

Complete library #3

Merged
merged 12 commits into from
Sep 10, 2023
Merged

Complete library #3

merged 12 commits into from
Sep 10, 2023

Conversation

RandomModderJDK
Copy link
Contributor

@RandomModderJDK RandomModderJDK commented Sep 10, 2023

Hi,
just wanted to use this project in another project, but I thought it would be nice if it would be modernized. So that's why I wanna try to make it to a package.
PS: At first, I wanted to code my own untis client, but you already did. Thx

A summary:
Added a pubspec.yaml
Applied package template
Reformatted file
(Hopefully) improved readability
Upped version requirements
Now in Flutter 3

@IsAvaible
Copy link
Owner

Hey @RandomModderJDK, thanks for your pull request! I looked through the changes you made, and it seems like a good idea to modernize the package structure.

My only nitpick is the reformatting of the README, as I think that the spreading out assignments / function calls over multiple lines, makes the code less readable. E.g.:

var myId = (await mySession.searchStudent("demo_forename", "demo_surname"))!.surnameMatches![0].id;

was reformatted to

var myId = (await mySession.searchStudent("demo_forename", "demo_surname"))!.surnameMatches!
[
0
]
.
id;

If you could revert the changes you made to the code snippets in the README, I would be happy to merge your pull request and publish a new release.

@RandomModderJDK
Copy link
Contributor Author

RandomModderJDK commented Sep 10, 2023

Hey @IsAvaible,
here you go. Btw. thanks for the fast response.

The package URL was intentionally prefixed with https:// but was changed to git:// when reverting the formatting changes. This commit re-applies the https:// prefix.
@IsAvaible IsAvaible merged commit c93e1c7 into IsAvaible:main Sep 10, 2023
@IsAvaible
Copy link
Owner

I have just merged your pull request and created a new release. Have fun coding your project!

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.

2 participants