Skip to content

Rename rojo project during upload when wally package name doesn't match#25

Merged
magnalite merged 6 commits intomainfrom
rojo-project-handling-soft-fix
Sep 29, 2021
Merged

Rename rojo project during upload when wally package name doesn't match#25
magnalite merged 6 commits intomainfrom
rojo-project-handling-soft-fix

Conversation

@magnalite
Copy link
Copy Markdown
Contributor

@magnalite magnalite commented Aug 23, 2021 •
edited
Loading

At the moment it is possible to publish a package which has a name differing from a name given in a Rojo default.project.json. This means Rojo will rename the package later on causing problems with linking. This PR aims to mitigate this by preventing users publishing packages with mismatched names and prompting them to rename.

It does so by checking if a default.project.json file exists in the project during publishing, and if it does, asserts the project name matches the package name given in the wally.toml.

This PR also adds:

  • A test to verify publishing will stop when names are mismatched (and users are given an appropriate error).
  • Another test to verify users are given an appropriate error when the names are fine (instead of a mismatched error).
  • An extra global argument to allow tests to specify a temporary package index should be used. This caused problems when multiple publish tests were being run.
  • The publish command now also accepts global arguments.
  • Renames the wally-test-index url in the test packages to the new one (instead of the old archived one which 404s).

@magnalite magnalite requested a review from LPGhatguy August 23, 2021 21:29
@magnalite magnalite linked an issue Aug 23, 2021 that may be closed by this pull request
Package name in wally.toml doesn't match name in project.json #14
Closed
@LPGhatguy
Copy link
Copy Markdown
Contributor

LPGhatguy commented Aug 30, 2021

There are some good changes in this PR! I think that we should shift the titular feature. Instead of validating that the user's default.project.json matches, we should just fix it for them in the zip that we upload. This is important because we can't or don't want to change the default.project.json for many projects because it impacts the name of the model when built by Rojo, like for standalone releases!

@magnalite
Copy link
Copy Markdown
Contributor Author

magnalite commented Sep 7, 2021

There are some good changes in this PR! I think that we should shift the titular feature. Instead of validating that the user's default.project.json matches, we should just fix it for them in the zip that we upload. This is important because we can't or don't want to change the default.project.json for many projects because it impacts the name of the model when built by Rojo, like for standalone releases!

Sounds like a good idea! I'll change this to modify the default.project.json during upload. I think we should still output a warning indicating that this is happening and recommend changing it to match if possible. Obviously that warning will not stop the upload, it'll just clarify what is happening.

@LPGhatguy
Copy link
Copy Markdown
Contributor

LPGhatguy commented Sep 7, 2021

I think we should still output a warning indicating that this is happening and recommend changing it to match if possible.

I don't know about this. For Roact, they probably want to keep the project name as Roact because this is important for rojo build. Changing the name to roact would be desirable for Wally but would break the normal releases.

I think we can have an info-level log message, but we shouldn't necessarily push people to change their Rojo projects.

@magnalite
Copy link
Copy Markdown
Contributor Author

magnalite commented Sep 8, 2021 •
edited
Loading

I think we can have an info-level log message, but we shouldn't necessarily push people to change their Rojo projects.

Sounds good to me. I've now changed this to detect+modify a default.project.json during creation of PackageContents and output a neutral info log. I'm not sure if this still counts as an integration test and I would have preferred to keep using the publish command but that felt like unnecessary complexity.

@magnalite magnalite changed the title Return appropriate error when rojo project and wally package names don't match Rename rojo project during upload when wally package name doesn't match Sep 8, 2021
@magnalite magnalite merged commit beeb9a8 into main Sep 29, 2021
@magnalite magnalite deleted the rojo-project-handling-soft-fix branch September 29, 2021 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LPGhatguy LPGhatguy Awaiting requested review from LPGhatguy

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Package name in wally.toml doesn't match name in project.json

2 participants

@magnalite @LPGhatguy