Skip to content

More Pirates#18

Draft
pingevt wants to merge 2 commits into
3.xfrom
feature/mo-pirates
Draft

More Pirates#18
pingevt wants to merge 2 commits into
3.xfrom
feature/mo-pirates

Conversation

@pingevt
Copy link
Copy Markdown
Member

@pingevt pingevt commented Jan 19, 2024

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR expands the bc_api_example Drupal module’s sample content by adding more pirate/nationality/ship seed data and updates the field_nickname field storage to allow multiple nickname values per pirate.

Changes:

  • Set field_nickname storage cardinality to unlimited (-1) to support multiple nicknames.
  • Expanded seeded nationalities and ships lists.
  • Updated/expanded seeded pirates so nicknames are provided as arrays (multi-value).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
modules/bc_api_example/config/optional/field.storage.node.field_nickname.yml Changes nickname field storage to unlimited cardinality.
modules/bc_api_example/bc_api_example.data.inc Adds more sample nationalities/ships/pirates and converts pirate nickname data to multi-value arrays.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

'france' => [
'label' => 'France',
'tid' => NULL,
],
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

bc_api_example_get_pirates_data() references nationality key ireland (e.g., Anne Bonny / Grace O'Malley), but bc_api_example_get_nationalities_data() does not define an ireland entry. This will trigger an undefined index when mapping $nationalities[$pn]['tid'] during hook_install(). Add ireland to the nationalities data (or change the pirates to use an existing key).

Suggested change
],
],
'ireland' => [
'label' => 'Ireland',
'tid' => NULL,
],

Copilot uses AI. Check for mistakes.
Comment on lines +238 to +282
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

There are multiple pirates defined with an empty array key ('' => [...]). In PHP, duplicate array keys will overwrite earlier entries, and these empty references (nationality/ships of '') will also break hook_install() lookups. Remove these placeholder entries or replace them with unique IDs and valid nationality/ship keys.

Suggested change
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],
'' => [
'nid' => NULL,
'status' => 1,
'title' => '',
'nickname' => ["", ""],
'body' => "",
'nationality' => [''],
'ships' => [''],
],

Copilot uses AI. Check for mistakes.
Comment on lines +175 to +179
'zheng yi sao' => [
'nid' => NULL,
'status' => 1,
'title' => 'Zheng Yi Sao',
'nickname' => ["The Terror of South China", "Dragon Lady of the Seas"],
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The pirate array key zheng yi sao contains spaces, while other IDs in this data set use snake_case machine names (e.g., black_beard, henry_every). For consistency and to avoid awkward referencing later, consider renaming this key to a snake_case identifier (and update any references if added).

Copilot uses AI. Check for mistakes.
'nid' => NULL,
],
'pirate_queen' => [
'title' => 'pirate_queen',
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

Ship titles look like human-readable labels elsewhere (e.g., "Royal Fortune", "Queen Anne's Revenge"), but pirate_queen is set to the machine name pirate_queen. If this is meant to be displayed, update it to a properly formatted title for consistency.

Suggested change
'title' => 'pirate_queen',
'title' => 'Pirate Queen',

Copilot uses AI. Check for mistakes.
@pingevt
Copy link
Copy Markdown
Member Author

pingevt commented Apr 5, 2026

@copilot apply changes based on the comments in this thread

…te key, remove empty placeholders

Agent-Logs-Url: https://github.com/bluecadet/bc_api_base/sessions/930d7a66-aaf8-4de8-ba04-d5994f3705cb

Co-authored-by: pingevt <3128823+pingevt@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented Apr 5, 2026

@copilot apply changes based on the comments in this thread

Applied all four fixes in commit 7118118:

  • Added ireland to bc_api_example_get_nationalities_data() so Anne Bonny and Grace O'Malley's nationality lookups resolve correctly
  • Fixed ship title 'pirate_queen''Pirate Queen'
  • Renamed pirate key zheng yi saozheng_yi_sao for snake_case consistency
  • Removed all five placeholder entries with empty array keys ('' => [...])

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