Implement server actions via an enum #144
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This moves the server actions to use a typed enum to represent the possible actions that can be performed on a server. This will make it easier to implement the rest of the actions in the future.
dtantsur
reviewed
Oct 7, 2023
dtantsur
reviewed
Oct 7, 2023
dtantsur
reviewed
Oct 7, 2023
dtantsur
reviewed
Oct 7, 2023
I'm not sure I want ALL OF THEM as methods. Maybe the most popular ones. This approach makes sense - please see the comments. |
It no longer needs to be Copy in this case, and shouldn't be anyway.
milliams
force-pushed
the
server_actions
branch
from
4fdffa5
to
9927dae
Compare
|
I think I've resolved the requested changes. Let me know if there's anything else. |
|
Sigh, it still fails because of missing_copy_implementation. I suggest you add an override for it (bonus for reporting it to the Rust team). |
As we will be adding more actions to this enum in the future, it will soon become non-Copyable so we don't want this lint triggering.
|
I've disabled the lint for now. As soon as I add some more actions (in a future PR), it will stop being needed (since they will be holding Also, bug Rust report raised at rust-lang/rust#116766. |
milliams
force-pushed
the
server_actions
branch
from
80a5b61
to
a4ecd39
Compare
|
Your CI patch is in a merge conflict now, could you split it away? |
milliams
force-pushed
the
server_actions
branch
from
a4ecd39
to
6c82cd3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves the server actions to use a typed enum to represent the possible actions that can be performed on a server.
In the future, I wouldn't expect that every single action would have a special method associated with it, so I opted for a generic
actionmethod which takes the new enum as an argument.This now allows you to do:
which is equivalent to (the still-present)
This is more verbose but will make it easier to implement the rest of the actions in the future as I would like to add the
createImageaction next, but I wanted to do the refactor separately first.I'm happy to take feedback on API and code structure, or of you think there's another way to implement it. For example, if you think that you'd rather have each future-implemented action done as a
Servermethod then this new approach gains less. Also, if the definition of theServerActionenum orunit_to_nullfunction make more sense in another file, I'm happy to move them.