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

remove unused addField code #8261

Conversation

RayBB
Copy link
Collaborator

@RayBB RayBB commented Sep 1, 2023

Contributes to #4474

It took me a little while to figure out that these fields were hidden and there wasn't any way to unhide them without modifying the code. However, I'm pretty sure that this is completely unused since it's probably rare that we add these and that @jimchamp just said he added one by editing the yaml in #8248 (which I've also done before).

I think it's probably best to remove this dead code but maybe @mekarpeles or @cdrini can say otherwise.

This helps us get very close the the epic to remove inline JS.

Testing

Double check that this removes all references.

Stakeholders

@jdlrobson

@codecov-commenter
Copy link

Codecov Report

Merging #8261 (870a024) into master (f7388d6) will increase coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8261      +/-   ##
==========================================
+ Coverage   16.97%   17.14%   +0.17%     
==========================================
  Files          72       71       -1     
  Lines        3789     3750      -39     
  Branches      654      647       -7     
==========================================
  Hits          643      643              
+ Misses       2728     2695      -33     
+ Partials      418      412       -6     
Files Changed Coverage Δ
openlibrary/plugins/openlibrary/js/index.js 0.00% <ø> (ø)

@jimchamp jimchamp merged commit c71ee55 into internetarchive:master Sep 6, 2023
@jimchamp
Copy link
Collaborator

jimchamp commented Sep 6, 2023

Thanks @RayBB!

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.

3 participants