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

Rename geo-compose -> geoview-compose #182

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

prupani-7
Copy link

@prupani-7 prupani-7 commented Mar 20, 2024

Description

PR to rename geo-compose -> geoview-compose

Links and Data

Sample Epic: runtime/kotlin/issues/3653

What To Review

  • Leaving out the renaming changes in tools/NewModuleScript to Ruiqi, as she will address that in [#3649]
  • Review the refactoring changes elsewhere

How to Test

Run the affected sample on the sample viewer or the repo.

@prupani-7
Copy link
Author

Seems like the style checker errors are false positives?
Folder path: analyze-hotspots

  1. analyze-hotspots - Error tags - Tags are not sorted.
  2. analyze-hotspots - Error checking extra tags due to previous error

The readme tags seem sorted, and so do the keywords in metadata json file

Copy link

@ruiqima ruiqima left a comment

Choose a reason for hiding this comment

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

One comment about tag reordering, others looks good 👍 Tested with sample viewer, it built successfully.

Context for other reviewers: updates related to NewModuleScript will be handled in a separate issue, as there requires some other changes too.

analyze-hotspots/README.md Outdated Show resolved Hide resolved
@prupani-7
Copy link
Author

Thanks Ruiqi, I easily overlooked that!

@eri9000 eri9000 self-requested a review March 21, 2024 20:22
Copy link
Collaborator

@eri9000 eri9000 left a comment

Choose a reason for hiding this comment

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

Good work @prupani-7
I see some more references to geocompose in MainScreenTemplate.kt and in .github/.../Main.yaml here.
do these need to be updated?

@prupani-7
Copy link
Author

prupani-7 commented Mar 21, 2024

Good work @prupani-7 I see some more references to geocompose in MainScreenTemplate.kt and in .github/.../Main.yaml here. do these need to be updated?

as mentioned above the MainScreenTemplate.kt update within the tools\NewModuleScript folder will be handled by Ruiqi's PR.

And the Main.yml has just a mention about the branch name which is still feature-branch/geo-compose
it doesn't reference the toolkit name.

@prupani-7 prupani-7 requested a review from eri9000 March 21, 2024 22:40
Copy link
Collaborator

@eri9000 eri9000 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@prupani-7 prupani-7 merged commit efc00f6 into feature-branch/geo-compose Mar 21, 2024
1 check failed
@prupani-7 prupani-7 deleted the priy/update-to-geoviewcompose branch March 21, 2024 22:42
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