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 database #39

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Conversation

JustinKyleJames
Copy link
Contributor

This is the code for the Metalnx database removal.

I haven't run the tests yet and they probably need some tweaking. I will probably get that in another pull request.

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good.

Please add more words to the commit body of each commit (where it makes sense) explaining the what/why. Having more info in the commits will help us later on if we ever need to circle back to these changes.

@korydraughn
Copy link
Collaborator

Anything else left to do for this PR?

@JustinKyleJames
Copy link
Contributor Author

Anything else left to do for this PR?

No, other than possible changes for unit tests. I haven't run them yet. I am going to look at them today.

@korydraughn
Copy link
Collaborator

I've never run the unit tests for this repo, so it's not clear me that they are important.

If you're confident that these changes work for metalnx, squash the commits to taste and we'll get this merged.

@JustinKyleJames
Copy link
Contributor Author

I've never run the unit tests for this repo, so it's not clear me that they are important.

If you're confident that these changes work for metalnx, squash the commits to taste and we'll get this merged.

I squashed them all down to one commit. (I saved off the original in a separate branch in case we want to go back to multiple commits.)

I put each specific change as a bullet in that one commit.

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're happy with it, pound it.

- Reimplemented UserDaoImpl.java to use Jargon calls to iRODS.
- Remove code that involves with favorites, bookmarks, templates, and profiles.
- Remove code for unused user fields.
- Remove references to hibernate and javax.persistence and all database update code.
- Moved UserDaoImpl.java and IRODSServices.java to avoid cyclic dependency.
- Add new required dependency for emc-metalnx-core to depend on jargon-zipservice and jargon-ticket.
- Change build->tasks to build->target as seems to be required due to maven update.
- Remove UserDao dependency on GenericDao.  Remove GenericDao and GenericDaoImpl as these have DB specific functionality.
@JustinKyleJames
Copy link
Contributor Author

If you're happy with it, pound it.

done

@alanking alanking merged commit adef656 into DICE-UNC:master Dec 5, 2024
@JustinKyleJames JustinKyleJames deleted the remove_database branch December 6, 2024 16:11
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