Skip to content

Pre-trim DSO string catalog numbers#4144

Merged
10110111 merged 1 commit intomasterfrom
faster-dso
Feb 20, 2025
Merged

Pre-trim DSO string catalog numbers#4144
10110111 merged 1 commit intomasterfrom
faster-dso

Conversation

@10110111
Copy link
Copy Markdown
Contributor

Description

Currently DSO catalog numbers that are not actually numbers but strings are constantly re-trimmed on use. This PR trims them on reading.

The patch seems trivial, but maybe I'm missing some reason not to trim them?

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • Housekeeping

How Has This Been Tested?

Test Configuration:

  • Operating system: Ubuntu 20.04
  • Graphics Card: Intel UHD Graphics 620

Checklist:

  • My code follows the code style of this project.
  • 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 (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@github-actions
Copy link
Copy Markdown

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

Copy link
Copy Markdown
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

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

Thanks!

@alex-w alex-w added this to the 25.1 milestone Feb 20, 2025
@alex-w alex-w added the subsystem: catalogs The issue is related to supported catalogs of planetarium... label Feb 20, 2025
Copy link
Copy Markdown
Member

@gzotti gzotti left a comment

Choose a reason for hiding this comment

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

Uh yes, sure!

@10110111 10110111 merged commit b1eea50 into master Feb 20, 2025
29 of 31 checks passed
@10110111 10110111 deleted the faster-dso branch February 20, 2025 11:07
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Feb 24, 2025
@github-actions
Copy link
Copy Markdown

Hello @10110111!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Mar 23, 2025
@github-actions
Copy link
Copy Markdown

Hello @10110111!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

subsystem: catalogs The issue is related to supported catalogs of planetarium...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants