Skip to content

Conversation

@ath0mas
Copy link
Contributor

@ath0mas ath0mas commented Apr 18, 2021

Platforms affected

iOS

Motivation and Context

Mixed default values for create between cli doc, cordova-android and cordova-ios.

Description

Package ids were mostly correct but not all, and same for project name. Suggested proper defaults:

  • id / package name: io.cordova.helloCordova
  • name / project name: Hello Cordova

See linked PRs: apache/cordova-cli#554, apache/cordova-android#1213

And

  • mimic code from cordova-android for createProject on how to init and deal with its parameters
  • remove useless check on project_parent as fs.ensureDirSync is used

(see each commit for detail on my steps)

Testing

npm test success

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@codecov-commenter
Copy link

codecov-commenter commented Apr 18, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@a3a3936). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1100   +/-   ##
=========================================
  Coverage          ?   74.88%           
=========================================
  Files             ?       13           
  Lines             ?     1724           
  Branches          ?        0           
=========================================
  Hits              ?     1291           
  Misses            ?      433           
  Partials          ?        0           
Impacted Files Coverage Δ
bin/templates/scripts/cordova/lib/Podfile.js 73.20% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3a3936...84a2eea. Read the comment docs.

@breautek breautek requested a review from erisu April 19, 2021 01:14
@ath0mas ath0mas force-pushed the feature/create-defaults-ios branch from 6663d7b to 5651b57 Compare November 6, 2023 22:54
@ath0mas ath0mas force-pushed the feature/create-defaults-ios branch from 5651b57 to 88d3d00 Compare November 6, 2023 22:59
@erisu
Copy link
Member

erisu commented Aug 18, 2025

@ath0mas

Sorry this took a bit longer to review and merge — I'd like to move things forward and get this PR ready.

How would you prefer to proceed?

  1. Rebase this existing PR
  2. Open a new PR yourself
  3. Or have me create a new PR instead

Here are the changes I'd recommend:

  • Update the package id/name to: org.apache.cordova.hellocordova

Even though this is just a template (and we expect developers to set their own package ID), this one feels more accurate to the Apache Cordova project. While there is a cordova.io domain, it isn't officially under Apache.

Regarding the hellocordova vs helloCordova casing — I'd suggest going with lowercase. This follows the convention used for Java packages and iOS app identifiers.

For references, I also submitted related PRs to cordova-android and cordova-app-hello-world to align with these changes:

@dpogue dpogue added this to the 8.0.0 milestone Aug 21, 2025
@ath0mas
Copy link
Contributor Author

ath0mas commented Aug 29, 2025

@erisu If you're okay with it, I'll let you create the new PR instead.

@dpogue dpogue mentioned this pull request Aug 29, 2025
1 task
@ath0mas ath0mas closed this Aug 29, 2025
@ath0mas ath0mas deleted the feature/create-defaults-ios branch August 29, 2025 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants