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

Remove redundant constants class #2914

Closed
GCHQDeveloper314 opened this issue Mar 1, 2023 · 8 comments · Fixed by #2927
Closed

Remove redundant constants class #2914

GCHQDeveloper314 opened this issue Mar 1, 2023 · 8 comments · Fixed by #2927
Assignees
Labels
enhancement Improvement to existing functionality/feature tech-debt Relates to Technical Debt
Milestone

Comments

@GCHQDeveloper314
Copy link
Member

The class CommonConstants.java has two static fields. One is unused and the other (UTF-8) is used in various places in Gaffer but can be replaced with an equivalent Java builtin.

Remove this class and replace references to its field with the equivalent Java builtin.

@GCHQDeveloper314 GCHQDeveloper314 added tech-debt Relates to Technical Debt beginner labels Mar 1, 2023
@GCHQDeveloper314 GCHQDeveloper314 added this to the post-v2.0_backlog milestone Mar 2, 2023
@ibrijesh
Copy link
Contributor

ibrijesh commented Mar 4, 2023

I am interested in contributing, Could you please assign me this task?

I would like to clarify the following changes that need to be made:

1.Remove the file named CommonConstants.java.
2.Replace the line import uk.gov.gchq.gaffer.commonutil.CommonConstants; with import java.nio.charset.StandardCharsets;.
3.Replace all instances of CommonConstants.UTF_8 with StandardCharsets.UTF_8.

@GCHQDeveloper314
Copy link
Member Author

Yes that is correct. You may also need to ensure the import order is correct after replacing the old import, as we have checkstyle rules for enforcing a clean order. You can run the checkstyle checks without needing to do a full build and test of the codebase by using mvn checkstyle:check.

@ibrijesh
Copy link
Contributor

ibrijesh commented Mar 7, 2023

When i am runniing this command mvn checkstyle:check I am getting below error

Screenshot from 2023-03-07 14-36-30

@GCHQDeveloper314
Copy link
Member Author

It looks like you will need to run mvn clean install -Pquick first, in order to build the project dependencies.

@ibrijesh
Copy link
Contributor

ibrijesh commented Mar 7, 2023

Getting this error now

Screenshot from 2023-03-07 15-06-27

@GCHQDeveloper314
Copy link
Member Author

Java 8 is required for Gaffer 1.22 versions. If you need to use Java 11 then checkout the v2-alpha branch and use that instead of the default develop branch. The v2-alpha branch is where we are doing development for Gaffer 2.0 and it supports Java 11.

@ibrijesh
Copy link
Contributor

ibrijesh commented Mar 8, 2023

I have succefully build project dependencies using command in mvn clean install -Pquick but changes in code failing mvn checkstyle:check . What rules i have to follow while making changes ?

@GCHQDeveloper314
Copy link
Member Author

When running the checks a list of checkstyle rule failures will be shown for each module which is failing. These are not summarised at the end, so you'll need to scroll up in order to see them. It may be easier to run the checks on the individual modules you've modified by using the -pl :module-name flag (e.g. -pl :store). You could also open a draft PR which will show us any checkstyle issues as part of our GitHub actions checks.

ibrijesh added a commit to ibrijesh/Gaffer that referenced this issue Mar 12, 2023
@GCHQDeveloper314 GCHQDeveloper314 linked a pull request Mar 13, 2023 that will close this issue
t92549 pushed a commit that referenced this issue Mar 22, 2023
* #2914 Issue: Remove redundant constants class

* Simplify charset

* Fix import

* Remove erroneous use of charset with JSONSerialiser.serialise

* Use StandardCharsets inbuilt function
---------

Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
@t92549 t92549 modified the milestones: post-v2.0_backlog, v1.23.0 Mar 22, 2023
@t92549 t92549 added the enhancement Improvement to existing functionality/feature label Mar 22, 2023
GCHQDeveloper314 added a commit that referenced this issue Mar 23, 2023
* #2914 Issue: Remove redundant constants class

* Simplify charset

* Remove erroneous use of charset with JSONSerialiser.serialise

* Use StandardCharsets inbuilt function
---------

Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
Signed-off-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
t92549 pushed a commit that referenced this issue Mar 24, 2023
* Gh-2914: Remove redundant constants class (#2927)

* #2914 Issue: Remove redundant constants class

* Simplify charset

* Remove erroneous use of charset with JSONSerialiser.serialise

* Use StandardCharsets inbuilt function
---------

Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
Signed-off-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>

* Copyright

* Fix diverged change to test

* Fix diverged change to exceptions

---------

Signed-off-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
Co-authored-by: Brijesh Yadav <brijeshyadavjthj@gmail.com>
t92549 added a commit that referenced this issue May 12, 2023
* Update docs links in README

* Revert "Update docs links in README"

This reverts commit 2d87bf3.

* gh-2559 added hasTrait, its handler and corresponding tests (#2561)

* gh-2559 added hasTrait, its handler and corresponding tests

* gh-2559 amended comment in HasTrait, added additional tests for the handler

* gh-2559 null check & tidy test

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

* Update links to Gaffer docs in all files (#2566)

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

* gh-2604: Deprecates Parquet (#2606)

* gh-2556: Add HBaseStore deprecated tag, javadocs and note to README (#2605)

* gh-2556: Add HBaseStore deprecated tag, javadocs and note to README

* gh-2556: Amend version number

* Upgrade Hazelcast version to 5.1.1 (#2612)

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

* Gh-2602: Replace Log4j with Reload4j (#2609)

* Replace Log4j with Reload4j
Also add enforcer plugin to prevent future problems

* FederatedStoreGetTraitsTest did not delete the cache service before carrying out each test causing it to fail when run as part of the maven tests. In isolation it passed. Found that the FederatedGraphStorageTraitsTest was also not clearing the cache. Added the before and after cache clearance and bumped the JUnit version of FederatedStoreGetTraitsTest from 4 to 5. Tests now pass (#2617)

Co-authored-by: r32575 <63105662+r32575@users.noreply.github.com>

* Gh 2562 GetElementsWithinSet not allowing for the exclusion of properties - Gaffer v1 (#2597)

* Fixed bug gh-2562. View properties are now being filtered. Added tests

* After a review moved the return statements inside the doPostFilter if-check and changed a conditional fail to an assertThat test

* Removed the try-finally blocks as the classes implement CloseableIterable

* Added failure message to test

* Adjusted javadoc formatting

* Removed rogue letter

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

* prepare release gaffer2-1.22.0

* prepare for next development iteration

* gh-2618: Cleanup inconsistent use of logging strings (#2651)

* Found 1 extra inconsistency than the mentioned one

* found and fixed other inconsitencies

* further fixes

* further fixes 2

* fixed 3 out of 4 checkstyle errors - 1 more to fix

* all checkstyle errors fixed

* removed use of String.format

* removed another instance of using String.format

* more changes

* additional change

* reverted changes

* replaced to SLF4J (#2663)

* gh-2669: Added new aggregation function endpoints (#2670)

* gh-2673: Fix links to koryphe core (#2674)

* Gh 2705 operation details spring (#2706)

* gh-2705: Cherry pick new spring endpoint

* gh-2696: Added tests for new all operation details endpoint (#2700)

* gh-2696: Added tests for new all operation details endpoint

* gh-2696: Spotless apply

* gh-2705: Copyright fix

* Gh-2745: Fix crown copyright link

Fixed Crown Copyright Link on Line 44

* Gh-2776: Create issue templates (#2777)

* Create issue templates

* Simplify bug report

* build(deps): bump hazelcast (#2848)

Bumps [hazelcast](https://github.com/hazelcast/hazelcast) from 5.1.1 to 5.1.3.
- [Release notes](https://github.com/hazelcast/hazelcast/releases)
- [Commits](hazelcast/hazelcast@v5.1.1...v5.1.3)

---
updated-dependencies:
- dependency-name: com.hazelcast:hazelcast
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* GH-2845: updt class var parameter to use Map interface (#2846)

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

* Gh-2914: Remove redundant constants class (#2927)

* #2914 Issue: Remove redundant constants class

* Simplify charset

* Fix import

* Remove erroneous use of charset with JSONSerialiser.serialise

* Use StandardCharsets inbuilt function
---------

Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>

* prepare release gaffer2-1.23.0

* prepare for next development iteration

* Fix field names in ToCsv

* Refactor to string enums

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
Co-authored-by: p3430233 <61738524+p3430233@users.noreply.github.com>
Co-authored-by: r32575 <63105662+r32575@users.noreply.github.com>
Co-authored-by: Gaffer <github-actions@github.com>
Co-authored-by: Robert <33911814+RobertG111@users.noreply.github.com>
Co-authored-by: Anirban Patra <96457415+AnirbanPatragithub@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: kvaithin <68995267+kvaithin@users.noreply.github.com>
Co-authored-by: Brijesh Yadav <brijeshyadavjthj@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to existing functionality/feature tech-debt Relates to Technical Debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants