Skip to content

CMR-9332: Fix lint issues for ACL libs#2225

Closed
DuJuan wants to merge 0 commit intomasterfrom
CMR-9332-fix-acl-lint-issues
Closed

CMR-9332: Fix lint issues for ACL libs#2225
DuJuan wants to merge 0 commit intomasterfrom
CMR-9332-fix-acl-lint-issues

Conversation

@DuJuan
Copy link
Copy Markdown
Contributor

@DuJuan DuJuan commented Mar 5, 2025

Overview

What is the feature/fix?

Resolves lint errors and warnings in ACL-related files, enhancing code quality and consistency across the codebase.

What is the Solution?

  1. Updating naming conventions (e.g., changing 'acl_core' to 'acl-core')
  2. Refactoring functions for better readability (e.g., search-for-groups, context->sids)
  3. Using more idiomatic Clojure functions (e.g., zero?, pos?, not-any?)
  4. Simplifying expressions and improving error handling
  5. Updating test files to reflect changes and improve mock functions

What areas of the application does this impact?

  • Access Control API routes
  • ACL validation services
  • ACL core functionality
  • ACL-related test files

Checklist

  • I have updated/added unit and int tests that prove my fix is effective or that my feature works
  • New and existing unit and int tests pass locally and remotely
  • clj-kondo has been run locally and all errors corrected
  • I have removed unnecessary/dead code and imports in files I have changed
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have cleaned up integration tests by doing one or more of the following:

@DuJuan DuJuan requested review from eereiter, jceaser and jmaeng72 March 5, 2025 01:19
Copy link
Copy Markdown
Contributor

@eereiter eereiter left a comment

Choose a reason for hiding this comment

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

The spacing is off for the routes.clj namespace.

Copy link
Copy Markdown
Contributor

@jceaser jceaser left a comment

Choose a reason for hiding this comment

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

I think the questions on spacing need to be answered for routes.clj

@jceaser jceaser added the code-quality Effort to improve the quality of the code label Mar 7, 2025
providers)]
{:status 200
:body (json/generate-string s3-list)}))
(defn- get-allowed-s3-buckets
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the spacing on this file is still off. Why is there a space in front of the func names?

(let [provider-id (acls/acl->provider-id acl)]
(when (and provider-id
(not (some #{provider-id} (map :provider-id (mdb/get-providers context)))))
(not-any? #{provider-id} (map :provider-id (mdb/get-providers context))))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is there a unit test on this func that makes sure the functionality of it has not changed?

@jmaeng72
Copy link
Copy Markdown
Contributor

let's have a chat about this, I think it would be easier than back and forth PR comments

@DuJuan DuJuan force-pushed the CMR-9332-fix-acl-lint-issues branch from d223e38 to 86e42d9 Compare March 14, 2025 20:15
@DuJuan DuJuan closed this Mar 14, 2025
@DuJuan DuJuan force-pushed the CMR-9332-fix-acl-lint-issues branch from 86e42d9 to 56bf291 Compare March 14, 2025 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-quality Effort to improve the quality of the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants