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.

Sign up for GitHub

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

Jump to bottom

Export should require specifying a conversation id or user. #1181

Open
metasoarous opened this issue Oct 24, 2021 · 3 comments · May be fixed by #1657
Open

Export should require specifying a conversation id or user. #1181

metasoarous opened this issue Oct 24, 2021 · 3 comments · May be fixed by #1657
Labels
feature-request For new feature suggestions 🐣 good-first-issue Good issues for first time contributors
Projects
Polis Development

Comments

@metasoarous
Copy link
Member

Problem:
When clj -M:run export is run without either -z, -Z or -u the program spits out an empty export file (this issue came up in discussion question #1180).

Suggested solution:
The program should make sure that one of these options is present, and print an error message if not.
@metasoarous metasoarous added the feature-request For new feature suggestions label Oct 24, 2021
@metasoarous metasoarous changed the title Please write a concise description of the problem here Export should require specifying a conversation id or user. Oct 24, 2021
@metasoarous metasoarous added this to Priority in Polis Development Dec 2, 2021
@metasoarous metasoarous added 🐣 good-first-issue Good issues for first time contributors 🙋🏾‍♀️ help wanted labels Dec 2, 2021
@thewindsofwinter
Copy link

I don't know if this is something still being worked on, but can I work on it? Just discovered this project today and it seems awesome, would love to contribute!

@metasoarous
Copy link
Member Author

@thewindsofwinter Sure! This would be very helpful. Thanks for the offer :-)

The place to look would be here: https://github.com/compdemocracy/polis/blob/edge/math/src/polismath/runner.clj#L211-L247

Look to see if the CLI option parsing library in use there allows us to specify that one of the two options be required. Otherwise, can just manually check to see if at least one of the options has been specified, and raise an error if not.

If you aren't familiar with Clojure, there are some links you may find helpful in the math/README. Note that the dev tooling for the math worker isn't quite working right now, so may find it easiest just to install clojure locally, and run from there to test. If you start up the rest of the system with make start-rebuild, you can use the DATABASE_URL in your .env file to connect from your Clojure to test things.

Feel free to reach out if you have any questions!

@thewindsofwinter thewindsofwinter linked a pull request Mar 9, 2023 that will close this issue
Open
@thewindsofwinter
Copy link

Made some edits: let me know how they look. Clojure is a strange new jungle, so I wouldn't be surprised if something is off: the docker builds fine but I'm not super familiar with the nREPL so testing hasn't been done thoroughly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-request For new feature suggestions 🐣 good-first-issue Good issues for first time contributors
Projects
Polis Development
Priority
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Resolves #1181 - Enforce User/Conversation ID thewindsofwinter/polis
3 participants
@metasoarous @colinmegill @thewindsofwinter