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

[imaging browser] Remove apparently unused button #9075

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

maximemulder
Copy link
Contributor

@maximemulder maximemulder commented Feb 20, 2024

Brief summary of changes

While browsing Loris code, I stumbled upon a button in view session that appears to be broken / dead code in the imaging browser:

  • The button points to the BrainBrowser/display.html URL. However, there seems is no such file in LORIS.
  • The button uses a relative link, which is probably wrong.
  • The button is only displayed if a session has a corresponding "obj" file in the database. However, there is no such file in Rainsinbread.

Thus, I open this PR to remove this button. But before merging it, I'd like to know if anyone has any information regarding the button or its display condition.

Pinging the imaging squad @ridz1208 @cmadjar @laemtl.

@maximemulder
Copy link
Contributor Author

maximemulder commented Feb 20, 2024

I probably should have looked at that earlier, but considering this button seems to have been untouched for more than 10 years, I'd say there is a 100% chance it needs to be removed. I am still curious about the "obj" files though, I don't know what those are.

@maximemulder maximemulder requested review from driusan and removed request for driusan February 21, 2024 15:11
@cmadjar
Copy link
Collaborator

cmadjar commented Mar 27, 2024

I am not sure that this button should be removed. "obj" files are specific derivatives image files (which raisin bread don't have). I don't know if some projects are still using it. I think this should be brought up at the next LORIS call to see if any project uses that feature.
@maximemulder can you add this to the next LORIS call agenda?

@maximemulder
Copy link
Contributor Author

Okay, added!

@maximemulder
Copy link
Contributor Author

maximemulder commented Apr 2, 2024

We discussed this subject in the latest LORIS meeting, and concluded no one depends on this button. The page display.html probably existed at some point, but it was removed a long time ago (before version 15). So we are in favor of removing this button @cmadjar (unless you have some more questions).

@cmadjar
Copy link
Collaborator

cmadjar commented Apr 3, 2024

Sounds good! @driusan ready for you :-)

@driusan driusan merged commit a5b5189 into aces:main Apr 4, 2024
28 checks passed
@ridz1208 ridz1208 added this to the 26.0.0 milestone Apr 9, 2024
@maximemulder maximemulder deleted the remove-unused-button branch October 29, 2024 17:02
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.

5 participants