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 old files #1282

Merged
merged 3 commits into from
Sep 6, 2021
Merged

Remove old files #1282

merged 3 commits into from
Sep 6, 2021

Conversation

ellert
Copy link
Contributor

@ellert ellert commented Sep 16, 2020

Consider at some point to remove files from the source tree that are no longer used.

@abh3
Copy link
Member

abh3 commented Sep 16, 2020

This is going to take some review to determine whether these deletions will impact our longer term plans. Normally, we leave "unused" parts because some people still depend on the, we have plans to reintegrate parts of them, provide templates for similar functionality, etc. Leaving the source versions in the repo is rather harmless even if somewhat ugly. What was the motivation for these deletions?

@ellert
Copy link
Contributor Author

ellert commented Sep 16, 2020

What was the motivation for these deletions?

I tried to phrase this carefully when I said "Consider at some point to remove". That is, I was not expecting this to be immediately accepted. The motivation was mainly to clean up the source distribution to not include dead code that is no longer used.

@abh3
Copy link
Member

abh3 commented Sep 16, 2020

That's OK, nothing wrong with eliminating dead code as it is ugly and can be confusing. It just means we need to review the pull request to extract the appropriate essence. I do appreciate that you provided a focus target which gives us an important starting point. So, I hope you don't mind that a review will ask for modifications of this pull request. I think this is worth pursuing to its conclusion.

@ellert
Copy link
Contributor Author

ellert commented Dec 4, 2020

Rebased because the "Remove unused parts of XrdOucCache" commit was no longer relevant after commit f853b74 that removed the same files.

@simonmichal
Copy link
Contributor

Just removed XrdClient: 644354d

@ellert
Copy link
Contributor Author

ellert commented Dec 7, 2020

Just removed XrdClient: 644354d

I have rebased again. The files changed count went down from 114 to 49.

The XrdCns would need some porting if it should be kept since it currently requires XrdClient that is now removed.

@simonmichal
Copy link
Contributor

XrdCns has been already removed from our builds, but lets wait for @abh3 for the final verdict.

@abh3
Copy link
Member

abh3 commented Dec 7, 2020 via email

@simonmichal
Copy link
Contributor

@ellert : just to let you know, I have removed XrdCns (actually the code has been moved to xrootd/archive repo).

@ellert
Copy link
Contributor Author

ellert commented Jan 7, 2021

@ellert : just to let you know, I have removed XrdCns (actually the code has been moved to xrootd/archive repo).

@simonmichal : You left the src/XrdCns.cmake file behind (and the out-commented include statement in src/CMakeLists.txt) 😀

@ellert
Copy link
Contributor Author

ellert commented Jan 7, 2021

I have rebased again. Now there are just 19 files left... (Down from 49.)

@simonmichal
Copy link
Contributor

@simonmichal : You left the src/XrdCns.cmake file behind (and the out-commented include statement in src/CMakeLists.txt) 😀

@ellert : thanks for pointing it out! I removed the left overs in: 9a0b4ec

@ellert
Copy link
Contributor Author

ellert commented Jan 21, 2021

And we are down to 15 files...

@simonmichal
Copy link
Contributor

I've just removed the obsolete man pages: 256873e

@ellert
Copy link
Contributor Author

ellert commented Apr 17, 2021

And then they were 11...

@simonmichal
Copy link
Contributor

@ellert : together with @abh3, we just did a review and the verdict is we want to keep following files:

XrdSecgsiAuthzFunDN.cc
XrdXrootdGPFile.hh
XrdPfcAllowDecision.cc 
XrdFrmAdminReloc.cc

Please modify your PR and then we can merge it.

@ellert
Copy link
Contributor Author

ellert commented Sep 6, 2021

Please modify your PR and then we can merge it.

I have modified the PR as requested.

@simonmichal simonmichal merged commit 3d88fba into xrootd:master Sep 6, 2021
@ellert ellert deleted the remove-old-files branch September 6, 2021 08:17
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.

None yet

3 participants