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

Fix code style and linting errors #223

Merged
merged 7 commits into from
Aug 22, 2024

Conversation

untari
Copy link
Contributor

@untari untari commented Aug 13, 2024

This PR contributes to resolving issues: Tests: Fix isort, flake8 code style, Postgres and Python tests

The changes ensure that the code adheres to the project's style guidelines, improving overall code quality and maintainability.

Testing:

  • All linting checks have been re-run after the fixes, and no errors or warnings are reported.
  • The application builds and runs as expected with the changes
    image

Summary by Sourcery

Fix code style and linting errors across the project to improve code quality and maintainability.

Enhancements:

  • Fix code style and linting errors across multiple Vue components and JavaScript files to adhere to project style guidelines.

Copy link

sourcery-ai bot commented Aug 13, 2024

Reviewer's Guide by Sourcery

This pull request focuses on improving code style and fixing linting errors across multiple files in the project. The changes primarily involve formatting adjustments, variable renaming for consistency, and minor syntax improvements to adhere to the project's style guidelines. Key modifications include:

  1. Consistent use of single quotes for strings
  2. Proper indentation and spacing
  3. Renaming variables to follow camelCase convention
  4. Removing unnecessary semicolons
  5. Reorganizing import statements
  6. Fixing minor syntax issues

These changes aim to enhance code readability, maintainability, and consistency throughout the codebase.

File-Level Changes

Files Changes
webapp/src/views/schedule/sessions/index.vue
webapp/src/views/schedule/export-select.vue
webapp/src/lib/quill/mention.js
webapp/src/lib/quill/emoji.js
webapp/config.js
Replaced double quotes with single quotes for string literals
webapp/src/views/schedule/sessions/index.vue
webapp/src/views/schedule/export-select.vue
webapp/src/lib/quill/mention.js
webapp/src/lib/profile.js
webapp/src/views/admin/rooms/types-edit/stage.vue
Adjusted indentation and spacing for improved readability
webapp/src/views/schedule/index.vue
webapp/src/components/MediaSource.vue
Renamed variables to follow camelCase convention (e.g., 'language_track' to 'languageTrack')
webapp/src/views/schedule/index.vue
webapp/src/components/MediaSource.vue
webapp/src/views/schedule/schedule-components/Session.vue
Removed unnecessary semicolons at the end of statements
webapp/src/views/rooms/item.vue
webapp/src/main.js
Reorganized and formatted import statements
webapp/src/theme.js
webapp/src/store/index.js
webapp/vue.config.js
Fixed minor syntax issues and improved code structure
webapp/src/views/schedule/talks/item.vue
server/venueless/importers/conftool.py
Ensured consistent naming of function parameters across files
webapp/src/components/janus/JanusConference.vue
webapp/src/components/janus/JanusVideoroom.vue
webapp/src/locales/index.js
Removed trailing whitespace and added newline at end of file

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @untari - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Potential hard-coded secret in authorization header. (link)
  • Potential hard-coded URL in QR code generation. (link)
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🔴 Security: 2 blocking issues
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

try {
this.isExporting = true
const url = config.api.base + 'export-talk?export_type=' + this.selectedExporter.id
const authHeader = api._config.token ? `Bearer ${api._config.token}` : (api._config.clientId ? `Client ${api._config.clientId}` : null)
Copy link

Choose a reason for hiding this comment

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

🚨 issue (security): Potential hard-coded secret in authorization header.

Using api._config.token and api._config.clientId directly in the code can expose sensitive information. Consider fetching these values from environment variables or a secure vault.

if (!['ics', 'xml', 'myics', 'myxml'].includes(option.id)) {
return
}
const url = config.api.base + 'export-talk?export_type=' + option.id
Copy link

Choose a reason for hiding this comment

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

🚨 issue (security): Potential hard-coded URL in QR code generation.

Using config.api.base directly in the code can expose sensitive information. Consider fetching this value from environment variables or a secure vault.

server/venueless/importers/conftool.py Outdated Show resolved Hide resolved
webapp/src/components/MediaSource.vue Outdated Show resolved Hide resolved
@hongquan
Copy link
Member

Please revert all the changes that adds whitespace between function name and (.

@untari
Copy link
Contributor Author

untari commented Aug 16, 2024

Hi @hongquan, I double-checked the upstream code, and it also uses a space between the function name and the parameters (). According to the ESLint documentation, I could adjust the rules to remove the space, but I want to clarify this with both the upstream code and the ESLint documentation. Currently, if I remove the space, it won't pass the ESLint checks.

@untari
Copy link
Contributor Author

untari commented Aug 16, 2024

fix tests it's to resolve flake8

@hongquan
Copy link
Member

@untari Could you help adjust ESLint configuration, removing "space-before-function-paren" rule?
Our JS code primarily targets VueJS, and we should follow VueJS style (no space between function name and "(", no semicolon at the end of line etc.).

Later on, I plan to use these tools for linting:

  • For Python code, use Ruff in place of flake8, isort.
  • For JS, reconfigure ESLint to follow VueJS style.

@untari
Copy link
Contributor Author

untari commented Aug 17, 2024 via email

@untari untari requested a review from hongquan August 20, 2024 06:59
@untari untari requested a review from hongquan August 22, 2024 14:33
@untari
Copy link
Contributor Author

untari commented Aug 22, 2024

Hi @mariobehling @hongquan could you review the latest changes for flake8, black, isort and lint. These tests are passed. If there is no further feedback, please merge this. So I don't have to keep fixing the new code that's not following the style guide and start to enforcing to use lint, flake8 isort and black tests. Thanks

@mariobehling mariobehling merged commit f2dce7e into fossasia:development Aug 22, 2024
6 of 8 checks passed
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