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

added support for NIP-62 Request to Vanish events #80 #1507

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Conversation

bryanmontz
Copy link
Contributor

Issues covered

#80

Description

Adds support for authoring NIP-62 Request to Vanish events.

Note: This is only the data layer. The UI changes required for the user to make this request will be added in a subsequent PR.

No UI changes.

Initially I added a function for the new event to CurrentUser.swift, but that triggered the 500-line file lint rule. So I made an extension on CurrentUser for all the publish event functions.

This PR also adds the concept of an extension on JSONEvent that could house easier-to-use APIs for event creation. It also improves testability.

Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks great! A couple questions:

If we see a request to vanish for a user, should we delete all events for that user? And if so, should that be in this PR or a separate one?

README.md Show resolved Hide resolved
Nos/Service/CurrentUser.swift Show resolved Hide resolved
@bryanmontz
Copy link
Contributor Author

This all looks great! A couple questions:

If we see a request to vanish for a user, should we delete all events for that user? And if so, should that be in this PR or a separate one?

I would think that would be in a separate PR, but yeah, we should do that.

@bryanmontz
Copy link
Contributor Author

@joshuatbrown Based on the title of this PR, you're probably right to assume that we should make your suggested change here. I still think it would be better to separate that though. Here's a ticket for it: #1511.

Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work again, @bryanmontz!

# Conflicts:
#	CHANGELOG.md
#	Nos.xcodeproj/project.pbxproj
@bryanmontz bryanmontz added this pull request to the merge queue Sep 18, 2024
Merged via the queue into main with commit 3bd4f82 Sep 18, 2024
4 checks passed
@bryanmontz bryanmontz deleted the bdm/80 branch September 18, 2024 12:30
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