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.

Sign up for GitHub

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

Jump to bottom

Rename dataRepo and org in the local storage #220

Merged
merged 4 commits into from
Nov 24, 2023
Merged

Rename dataRepo and org in the local storage #220

merged 4 commits into from
Nov 24, 2023

Conversation

wweqg
Copy link
Contributor

@wweqg wweqg commented Nov 21, 2023 •
edited
Loading

Summary:

Fixes #106

Type of change:

  • 🎨 Code Refactoring

Changes Made:

Rename dataRepo and org in the local storage to WATcher:dataRepo and WATcher:org

Proposed Commit Message:

Rename dataRepo and org to WATcher:dataRepo and WATcher:org

This commit changes the names of the data repository and the organization
to include the 'WATcher' prefix. The 'dataRepo' is now 'WATcher:dataRepo' 
and the organization name is changed to 'WATcher:org'.

Checklist:
  • I have tested my changes thoroughly.
  • I have created tests for any new code files created in this PR or provided a link to a issue/PR that addresses this.
  • I have added or modified code comments to improve code readability where necessary.
  • I have updated the project's documentation as necessary.

chunweii
chunweii requested changes Nov 23, 2023
View reviewed changes
Copy link
Contributor

@chunweii chunweii left a comment

Choose a reason for hiding this comment

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

Thank you for your PR @wweqg. It would be better to extract all the magic string literals into a constant instead

src/app/core/services/phase.service.ts
@@ -125,9 +125,9 @@ export class PhaseService {
* Retrieves the repository url from local storage and sets to current repository.
*/
async initializeCurrentRepository() {
const org = window.localStorage.getItem('org');
const repoName = window.localStorage.getItem('dataRepo');
this.logger.info(`PhaseService: received initial org (${org}) and initial name (${repoName})`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not change the logging message in the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I have updated it accordingly.

@wweqg
Copy link
Contributor Author

wweqg commented Nov 23, 2023

Thank you for your PR @wweqg. It would be better to extract all the magic string literals into a constant instead

Thank @chunweii for the comment. I have extracted the string literals into constants.

@chunweii
Copy link
Contributor

Please follow the git commit message conventions in https://se-education.org/guides/conventions/git.html

In particular, please have a proper subject, and a body

@wweqg
Copy link
Contributor Author

wweqg commented Nov 24, 2023

Please follow the git commit message conventions in https://se-education.org/guides/conventions/git.html

In particular, please have a proper subject, and a body

Thanks for the reminder. I have updated the proposed commit message

@chunweii chunweii merged commit ec22b01 into CATcher-org:main Nov 24, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chunweii chunweii chunweii approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use different names for dataRepo and org in local storage
2 participants
@wweqg @chunweii