Skip to content

refactor: migrate names-generator#1285

Merged
Azgaar merged 6 commits into
Azgaar:masterfrom
SheepFromHeaven:refactor/migrate-names-generator
Jan 27, 2026
Merged

refactor: migrate names-generator#1285
Azgaar merged 6 commits into
Azgaar:masterfrom
SheepFromHeaven:refactor/migrate-names-generator

Conversation

@SheepFromHeaven

@SheepFromHeaven SheepFromHeaven commented Jan 26, 2026

Copy link
Copy Markdown
Collaborator

Description

Type of change

  • Bug fix
  • New feature
  • Refactoring / style
  • Documentation update / chore
  • Other (please describe)

Versioning

  • Version is updated
  • Changed files hash is updated

@netlify

netlify Bot commented Jan 26, 2026

Copy link
Copy Markdown

Deploy Preview for afmg ready!

Name Link
🔨 Latest commit 89cc344
🔍 Latest deploy log https://app.netlify.com/projects/afmg/deploys/6978b286e1f8300008e3ca30
😎 Deploy Preview https://deploy-preview-1285--afmg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot AI left a comment

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.

Pull request overview

This pull request migrates the names-generator module from JavaScript to TypeScript, converting it from an IIFE pattern to a class-based implementation while maintaining the same public API through the global Names object.

Changes:

  • Migrated names-generator.js to TypeScript with proper type definitions for NameBase and MarkovChain
  • Added type declarations to global.ts for nameBases, mapName, and utility functions
  • Updated module import order to ensure names-generator loads before dependent modules
  • Removed the old JavaScript file reference from index.html

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/modules/names-generator.ts Complete TypeScript migration of the names generator module with class-based implementation
src/types/global.ts Added type declarations for names-related global variables and utility functions
src/types/PackedGraph.ts Added cultures property to PackedGraph interface
src/modules/index.ts Reordered imports to load names-generator before lakes and river-generator
src/index.html Removed old JavaScript file reference for names-generator
Comments suppressed due to low confidence (4)

src/modules/names-generator.ts:28

  • Extra leading space in the indentation. This line has an extra space before the for keyword that is inconsistent with the surrounding code formatting.
    src/modules/names-generator.ts:56
  • This comparison appears to have incorrect logic. The expression isVowel(that) === (next as unknown as boolean) casts the next string to boolean, which will always be truthy for non-empty strings. The original logic should compare if both characters are vowels. The correct logic should be: isVowel(that) === isVowel(next)
    src/modules/names-generator.ts:279
  • Spelling error in comment: "generato" should be "generate"
    src/modules/names-generator.ts:280
  • Missing return type annotation for the getMapName method. The method can return undefined (line 281-282) or "" (line 286), but no return type is specified. The return type should be explicitly declared as void | string or the method should be refactored to have consistent return behavior.

Comment thread src/types/global.ts Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@Azgaar Azgaar left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Has to be manually tested a bit.

Comment thread src/modules/index.ts Outdated
@Azgaar Azgaar merged commit 260ccd7 into Azgaar:master Jan 27, 2026
7 checks passed
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