Merged
Conversation
artlowel
added
the
1 APPROVAL
|
This pull request introduces 1 alert and fixes 1 when merging 21e8879 into cc618eb - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request fixes 3 alerts when merging bbc7844 into cc618eb - view on LGTM.com fixed alerts:
|
tdonohue
approved these changes
Aug 31, 2020
Member
tdonohue
left a comment
tdonohue
left a comment
There was a problem hiding this comment.
👍 Looks good to me. I've reviewed the code changes and it is just moving existing code around. I've also tested a build (yarn start) and I no longer see the circular dependency warnings (previously I did). The app looks to work fine as well...no obvious changes in behavior.
Merging immediately as this was flagged for 1 approval. Thanks @artlowel !
5 tasks
kosarko
pushed a commit
to ufal/dspace-angular
that referenced
this pull request
May 14, 2025
kosarko
pushed a commit
to ufal/dspace-angular
that referenced
this pull request
Jun 25, 2025
* Updated the no file preview message (DSpace#837) * Added spacing between clarin & dspace logo (DSpace#848) * Do not load Seznam license every time (DSpace#844) * Fixed encoding of the filename from the URL (DSpace#838) (DSpace#851) * Do not mount the Solr configs; copy them each time instead. (DSpace#850) * Fix the bulk access (DSpace#852) * Video files previews (#30)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
References
N/A
Description
Ever since the switch to angular cli we've been getting a lot of circular dependency warnings. The issues were always there, but CLI just started warning us about them.
This PR fixes those. They were almost all due to having the route constants and getter functions being defined in the same file in the routing module. Which caused all routing modules to depend on one another. Moving them to separate routing-paths files solved the issue.
This PR only moves things around and renames them for consistency, there is no new code. I haven't added tests or docs for things that didn't have them already.
Instructions for Reviewers
You can test it by verifying that the circular dependency warnings you get in the main branch on startup are gone, and that it introduces no new bugs.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
yarn run lintpackage.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.