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

refactor: first steps towards more independent packages (DSP-513) #1678

Merged
merged 24 commits into from Jul 27, 2020

Conversation

subotic
Copy link
Collaborator

@subotic subotic commented Jul 14, 2020

  • This PR started as fixing the twirl templates not showing correctly in IntelliJ but then took a turn
  • The PR became about refactoring of the packages so that they can be compiled independently because I initially thought that I need to compile the twirl-templates on their own.
  • To compile packages independently, I needed to resolve any circular dependencies.
  • Unfortunately, this still doesn't fix the IntelliJ problem, for which, I will open a new PR.

@subotic subotic self-assigned this Jul 14, 2020
@subotic subotic added build chore maintenance and build tasks labels Jul 14, 2020
@subotic subotic changed the title build: fix twirl sources not showing in intellij refactor: first steps towards more independent packages Jul 21, 2020
@subotic subotic marked this pull request as ready for review July 21, 2020 21:41
@subotic
Copy link
Collaborator Author

subotic commented Jul 22, 2020

@SepidehAlassi I've updated the documentation with the new package names.

@subotic subotic changed the title refactor: first steps towards more independent packages refactor: first steps towards more independent packages (DSP-513) Jul 23, 2020
Copy link
Contributor

@SepidehAlassi SepidehAlassi left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me, I have only a few tiny remarks.
PS. The calls of twirl templates with org.knora.webapi.messages.twirl. added to them are now inconveniently long. Is there any way to make them shorter?

webapi/src/main/scala/org/knora/webapi/http/BUILD.bazel Outdated Show resolved Hide resolved
tools/buildstamp/BUILD.bazel Outdated Show resolved Hide resolved
docs/05-internals/design/api-v2/overview.md Show resolved Hide resolved
@subotic
Copy link
Collaborator Author

subotic commented Jul 26, 2020

Looks mostly good to me, I have only a few tiny remarks.
PS. The calls of twirl templates with org.knora.webapi.messages.twirl. added to them are now inconveniently long. Is there any way to make them shorter?

Thanks for the review.

Yes, we can shorten them. The files generated through the twirl templates are normal scala methods part of a package. By importing the package, we could shorten the call to the method. Before they were part of a different package, but since they are part of a certain compilation unit, they should also be part of a package inside this compilation unit.

At the moment I would recommend leaving the naming with the fully qualified name because these methods are generated. They can only be moved to another package through search and replace, which is easier with their full name.

Wenn the twirl IntelliJ problem is fixed, we can then shorten them. IntelliJ will probably do it automatically.

@subotic subotic merged commit 830de4f into develop Jul 27, 2020
@subotic subotic deleted the wip/DSP-475-intellij-twirl-templates branch July 27, 2020 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore maintenance and build tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants