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

[#39] Fix two sonar cloud issues #45

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

panagiotisbellias
Copy link

@panagiotisbellias panagiotisbellias commented Apr 10, 2024

Description

Fix two sonar cloud issues removing commented out code and adding caption in a table html element

Related Issue

Motivation and Context

Reduces sonar cloud issues

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added

@koebel
Copy link
Collaborator

koebel commented Apr 11, 2024

@panagiotisbellias can you please check why drone is failing after your changes and try to fix this?

Copy link

sonarcloud bot commented Apr 11, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@panagiotisbellias
Copy link
Author

@panagiotisbellias can you please check why drone is failing after your changes and try to fix this?

@koebel drone was failing due to eslint check so I added a description to the table in another way but it may be presented in the UI, if this is not intended I can remove it completely until another solution comes up

@@ -25,7 +25,8 @@
</oc-button>
</div>
<div v-if="isMetadataExtracted" id="dicom-metadata-sidebar-content" class="oc-p-s">
<table class="details-table">
<p id="mydesc">DICOM metadata details table</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is your reasoning for adding the id? where is it used?

Copy link
Author

Choose a reason for hiding this comment

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

it is used in the table element

@koebel
Copy link
Collaborator

koebel commented Apr 11, 2024

@panagiotisbellias can you please check why drone is failing after your changes and try to fix this?

@koebel drone was failing due to eslint check so I added a description to the table in another way but it may be presented in the UI, if this is not intended I can remove it completely until another solution comes up

@panagiotisbellias can you please check yourself if the changes your suggesting affect the UI and if so, let me know who instructed you to make these UI changes?

@panagiotisbellias
Copy link
Author

panagiotisbellias commented Apr 11, 2024

@panagiotisbellias can you please check why drone is failing after your changes and try to fix this?

@koebel drone was failing due to eslint check so I added a description to the table in another way but it may be presented in the UI, if this is not intended I can remove it completely until another solution comes up

@panagiotisbellias can you please check yourself if the changes your suggesting affect the UI and if so, let me know who instructed you to make these UI changes?

I'm sorry, but no instructions are provided to run the VueJS part. Please add instructions (e.g. in README) first otherwise leave this sonar cloud issue unsolved until new solution comes up

Thanks,
Panagiotis

@koebel
Copy link
Collaborator

koebel commented Apr 11, 2024

@panagiotisbellias can you please check why drone is failing after your changes and try to fix this?

@koebel drone was failing due to eslint check so I added a description to the table in another way but it may be presented in the UI, if this is not intended I can remove it completely until another solution comes up

@panagiotisbellias can you please check yourself if the changes your suggesting affect the UI and if so, let me know who instructed you to make these UI changes?

I'm sorry, but no instructions are provided to run the VueJS part. Please add instructions (e.g. in README) first otherwise leave this sonar cloud issue unsolved until new solution comes up

Thanks, Panagiotis

If you are interested in running this app, have a look at the documentation of ownCloud's web extension system at https://owncloud.dev/clients/web/extension-system/

Basically you need to have an instance of ocis web and the ocis backend running, then build and run the extension with

pnpm install
pnpm build:w

and optionally

pnpm vite

then copy the config into the "external_apps" section of config of your ocis web instance (or create such a section if there isn't any)

Hope that helps

@CLAassistant
Copy link

CLAassistant commented Nov 25, 2024

CLA assistant check
All committers have signed the CLA.

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.

[Bug]: fix issues detected by sonar cloud
3 participants