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

Support for Bark (APN) notifications #782

Merged
merged 2 commits into from
Oct 26, 2021
Merged

Support for Bark (APN) notifications #782

merged 2 commits into from
Oct 26, 2021

Conversation

Lakr233
Copy link
Contributor

@Lakr233 Lakr233 commented Oct 23, 2021

Tested

IMG_A2C473708A92-1

@Lakr233
Copy link
Contributor Author

Lakr233 commented Oct 23, 2021

#457

@deefdragon
Copy link
Contributor

I am still working my way through them on my own PR, but please also write tests for the notification. You can see examples in #760 of how I am writing the tests, and what tests I am writing.

I would ask that you are a bit more through for this new notification, testing your helpers etc. on top of send.

@louislam louislam linked an issue Oct 24, 2021 that may be closed by this pull request
@Lakr233
Copy link
Contributor Author

Lakr233 commented Oct 25, 2021

I'd prefer you to pick my pull request and merge into your branch, so we can keep the commits nice and clean.

It's a little bit hard for me to understand the project architecture 🌚

@Lakr233
Copy link
Contributor Author

Lakr233 commented Oct 25, 2021

Bark requires a self-hosted-backend and an iOS device that generates APN token to work together, and I don't know how to setup a mock backend in your workflow...

@deefdragon
Copy link
Contributor

Its not going to be the easiest to keep things clean as I still have a few days of work to do. For now, just start with the send function tests, making sure axios gets called with the expected data based on the input data. (Relevant example)

Apply the changes I made to config/jest-backend.config.js and use npm run jest-backend to run them. As its going to take a few more days for me to finish the rest of the test, I will handle the merge conflict once this gets pulled in.

@chakflying
Copy link
Collaborator

BTW I'm not super experienced in Open Source, but is it an accepted practice to leave a comment like that in a MIT licensed project?

@deefdragon
Copy link
Contributor

is it an accepted practice to leave a comment like that in a MIT licensed project?

Can you provide context as to the exact comment you are referencing @chakflying, and why you are asking about it?

@Lakr233
Copy link
Contributor Author

Lakr233 commented Oct 26, 2021

BTW I'm not super experienced in Open Source, but is it an accepted practice to leave a comment like that in a MIT licensed project?

If you mean the comment generated by my code editor saying "all right reserved", then, I did not change the license file in the project which means it is still under MIT licensen.

@Lakr233
Copy link
Contributor Author

Lakr233 commented Oct 26, 2021

As its going to take a few more days for me

I'd like to wait for it, then check how can I write the test.

@deefdragon
Copy link
Contributor

deefdragon commented Oct 26, 2021

BTW I'm not super experienced in Open Source, but is it an accepted practice to leave a comment like that in a MIT licensed project?

If you mean the comment generated by my code editor saying "all right reserved", then, I did not change the license file in the project which means it is still under MIT licensen.

This SO question appears to be asking the same thing: https://softwareengineering.stackexchange.com/questions/117572/mit-and-copyright

and it looks like the consensus is that marking your copyright like this is fine (you automatically hold copyright for all works you are the authoring entity of anyway), and that it just also falls under MIT's clauses. You can still claim the copyright of that code, but it doesn't mean all that much, as its now part of the project. Someone else can use/edit it as MIT allows etc.

As its going to take a few more days for me

I'd like to wait for it, then check how can I write the test.

I guess I have some tests to finish then.

@louislam
Copy link
Owner

Off topic, I found that your status is very special, how did you get this?👀

image

@Lakr233
Copy link
Contributor Author

Lakr233 commented Oct 26, 2021

Off topic, I found that your status is very special, how did you get this?👀

image

截屏2021-10-26 上午10 36 06

@louislam
Copy link
Owner

Oh, got it. Change it to "Busy" and it becomes yellow.

@louislam louislam added this to the 1.10.0 milestone Oct 26, 2021
@louislam louislam merged commit d1c4d13 into louislam:master Oct 26, 2021
@Lakr233 Lakr233 deleted the dev-lakr233-bark_notification branch October 26, 2021 08:26
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.

[Feature Request] Bark notification support (iOS + APN)
4 participants