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: Rename Shortcode everywhere and use its type in KnoraProject property (NO-TICKET) #2724

Merged

Conversation

seakayone
Copy link
Collaborator

@seakayone seakayone commented Jun 30, 2023

Pull Request Checklist

Task Description/Number

Issue Number: DEV-

Basic Requirements

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • fix: represents bug fixes
  • refactor: represents production code refactoring
  • feat: represents a new feature
  • docs: documentation changes (no production code change)
  • chore: maintenance tasks (no production code change)
  • test: all about tests: adding, refactoring tests (no production code change)
  • other... Please describe:

Does this PR introduce a breaking change?

  • Yes
  • No
  • Maybe (not 100% sure => check with FE)

Does this PR change client-test-data?

  • Yes (don't forget to update the JS-LIB team about the change)
  • No

@seakayone seakayone changed the title Rename Shortcode and use in KnoraProject refactor: Rename Shortcode everywhere and use type in KnoraProject property (NO-TICKET) Jun 30, 2023
@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Patch coverage: 68.00% and project coverage change: +64.39 🎉

Comparison is base (12402f3) 18.00% compared to head (0021688) 82.39%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2724       +/-   ##
===========================================
+ Coverage   18.00%   82.39%   +64.39%     
===========================================
  Files         281      280        -1     
  Lines       28899    28899               
===========================================
+ Hits         5202    23812    +18610     
+ Misses      23697     5087    -18610     
Impacted Files Coverage Δ
...rg/knora/webapi/routing/admin/ProjectsRouteZ.scala 91.42% <0.00%> (ø)
...org/knora/webapi/routing/v1/ResourcesRouteV1.scala 2.00% <0.00%> (+2.00%) ⬆️
...ice/admin/api/service/ProjectsADMRestService.scala 60.00% <0.00%> (ø)
...i/slice/admin/domain/service/DspIngestClient.scala 4.76% <0.00%> (ø)
...ce/admin/domain/service/ProjectExportService.scala 68.18% <0.00%> (ø)
...ce/admin/domain/service/ProjectImportService.scala 44.89% <0.00%> (+32.65%) ⬆️
...ebapi/store/cache/impl/CacheServiceInMemImpl.scala 100.00% <ø> (+21.27%) ⬆️
...n/domain/service/ProjectExportStorageService.scala 26.08% <28.57%> (+3.35%) ⬆️
webapi/src/main/scala/dsp/valueobjects/Id.scala 34.61% <100.00%> (ø)
...bapi/src/main/scala/dsp/valueobjects/Project.scala 100.00% <100.00%> (+6.25%) ⬆️
... and 4 more

... and 187 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@seakayone seakayone force-pushed the refactor/replace-string-with-shortcode-type-in-knora-project branch 2 times, most recently from e14fba0 to 02bdb48 Compare June 30, 2023 11:25
@seakayone seakayone changed the title refactor: Rename Shortcode everywhere and use type in KnoraProject property (NO-TICKET) refactor: Rename Shortcode everywhere and use its type in KnoraProject property (NO-TICKET) Jun 30, 2023
@seakayone seakayone force-pushed the refactor/replace-string-with-shortcode-type-in-knora-project branch from 02bdb48 to 80623b8 Compare June 30, 2023 11:40
@seakayone seakayone marked this pull request as ready for review June 30, 2023 16:46
@seakayone seakayone self-assigned this Jun 30, 2023
Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Choose a reason for hiding this comment

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

We at some point took the conscious decision that we wanted to change shortcode to camel case (I think there must be a notion page somewhere, where we recorded naming decisions), but we never really tackled it, and it's probably easier to just keep the old capitalization, so I'm absolutely fine with this.
I think the same applies for shortname too, which then probably should be changed as well.

Copy link
Collaborator

@mpro7 mpro7 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mpro7
Copy link
Collaborator

mpro7 commented Jul 3, 2023

We at some point took the conscious decision that we wanted to change shortcode to camel case (I think there must be a notion page somewhere, where we recorded naming decisions), but we never really tackled it, and it's probably easier to just keep the old capitalization, so I'm absolutely fine with this. I think the same applies for shortname too, which then probably should be changed as well.

Good point and easy cheese: #2733

@seakayone seakayone merged commit f01c319 into main Jul 3, 2023
13 checks passed
@seakayone seakayone deleted the refactor/replace-string-with-shortcode-type-in-knora-project branch July 3, 2023 10:58
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