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

cookie-parser usage #947

Closed
huss opened this issue Jun 20, 2023 · 4 comments · Fixed by #991
Closed

cookie-parser usage #947

huss opened this issue Jun 20, 2023 · 4 comments · Fixed by #991
Assignees
Labels
assigned the issue is currently being worked on by a developer i-good-first-issue This issue is probably a good starting point for people new to coding or the OED project. p-medium-priority
Milestone

Comments

@huss
Copy link
Member

huss commented Jun 20, 2023

Is your feature request related to a problem? Please describe.

While working on PR #942, it was unclear if the package was actually used by OED. The only use is in src/server/app.js and I could not find any place that cookie was used elsewhere. OED does not seem to set any cookies. I commented it out and did not find any issues. Someone should see if it is really used (and hopefully with more knowledge of this than I have).

Describe the solution you'd like

Remove package if not used.

Describe alternatives you've considered

It stays in OED.

Additional context

None

@huss huss added p-medium-priority i-good-first-issue This issue is probably a good starting point for people new to coding or the OED project. labels Jun 20, 2023
@huss huss added this to the 1.1 release milestone Jun 20, 2023
@salhernandez
Copy link

@huss I would like for this issue to be assigned to me so that the students I'm mentoring for CodeDay Labs can work on it.

@huss huss added the assigned the issue is currently being worked on by a developer label Jul 6, 2023
@huss
Copy link
Member Author

huss commented Jul 6, 2023

@huss I would like for this issue to be assigned to me so that the students I'm mentoring for CodeDay Labs can work on it.

Done. I look forward to your contributions. Please let me know if you need any help.

@spearec
Copy link
Contributor

spearec commented Jul 13, 2023

@salhernandez I'm not sure if it will be related, but #958 adds a new user-specific language setting. Currently, this is set to the site default every time OED is loaded, but it could be useful to store this value in a cookie, so the user's preference is saved when they reload the site.
Even if OED chooses to remove this package for now, it would be good to have documentation on how to add it back in if we decide we want to start using cookies.

@salhernandez
Copy link

@spearec Thanks for the info!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned the issue is currently being worked on by a developer i-good-first-issue This issue is probably a good starting point for people new to coding or the OED project. p-medium-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants