Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ShippingService.php #753

Merged
merged 3 commits into from
Aug 5, 2024
Merged

Update ShippingService.php #753

merged 3 commits into from
Aug 5, 2024

Conversation

simonsolutions
Copy link
Contributor

RequiresAdditionalSellerInputs is not always present in response, so allow nulled value

RequiresAdditionalSellerInputs is not always present in response, so allow nulled value
@KhorneHoly
Copy link

KhorneHoly commented Jul 2, 2024

You edited a file that is auto generated. Have a look at this pull request: #741
I've had a similar issue with a different class. You should add the change to the modifications.json and then regenerate the files.

It's explained in the contributing.md.
Else your change would be revoked with the next generation process.

remove required parameter requiresAdditionalSellerInputs
Add removal of requirement "RequiresAdditionalSellerInputs" to modifications json
@simonsolutions
Copy link
Contributor Author

Thank you @KhorneHoly for stating out that dependency. I changed like in your PR both json files as well. Hope the change is complete in that way?!

@jlevers
Copy link
Owner

jlevers commented Jul 9, 2024

@simonsolutions this looks good. It looks like you modified the JSON schema file directly and then added the modification to the modifications.json file, which is fine, but I just would like to test the addition to modifications.json by running the commands below, and ensuring that your resources/models/seller/merchant-fulfillment/v0.json and src/Seller/MerchantFulfillmentV0/Dto/ShippingService.php files stay the same:

$ php bin/console schema:refactor --schema merchant-fulfillment
$ php bin/console schema:generate --schema merchant-fulfillment
$ composer lint

@simonsolutions
Copy link
Contributor Author

In which configuration do you run these commands, within my project the console command is not found and on a separate folder for the repo the autoload is not found.

@jlevers
Copy link
Owner

jlevers commented Jul 9, 2024

Are you in the root project directory for your fork of this repo when you run the commands?

@simonsolutions
Copy link
Contributor Author

I tried both, my project root and the fork's root directory

@jlevers
Copy link
Owner

jlevers commented Jul 9, 2024

Can you show me the error you're getting?

@simonsolutions
Copy link
Contributor Author

simonsolutions commented Jul 9, 2024

In my project root (this library added)

 There are no commands defined in the "schema" namespace.

  Did you mean this?
      doctrine:schema

and in the library's root:

PHP Warning:  require(/xxxxx/vendor/jlevers/selling-partner-api/bin/../vendor/autoload.php): Failed to open stream: No such file or directory in /xxxxx/vendor/jlevers/selling-partner-api/bin/console on line 3

Warning: require(/xxxxx/vendor/jlevers/selling-partner-api/bin/../vendor/autoload.php): Failed to open stream: No such file or directory in /xxxxx/vendor/jlevers/selling-partner-api/bin/console on line 3
PHP Fatal error:  Uncaught Error: Failed opening required '/xxxxx/vendor/jlevers/selling-partner-api/bin/../vendor/autoload.php' (include_path='.:/opt/homebrew/Cellar/php/8.3.9/share/php/pear') in /xxxxx/vendor/jlevers/selling-partner-api/bin/console:3
Stack trace:
#0 {main}
  thrown in /xxxxx/vendor/jlevers/selling-partner-api/bin/console on line 3

Fatal error: Uncaught Error: Failed opening required '/xxxxx/vendor/jlevers/selling-partner-api/bin/../vendor/autoload.php' (include_path='.:/opt/homebrew/Cellar/php/8.3.9/share/php/pear') in /xxxxx/vendor/jlevers/selling-partner-api/bin/console:3
Stack trace:
#0 {main}
  thrown in /xxxxx/vendor/jlevers/selling-partner-api/bin/console on line 3

@jlevers
Copy link
Owner

jlevers commented Jul 9, 2024

It looks like you're trying to run the command from inside this library's folder in the vendor/ directory of your project. You need to actually clone this repository, install the dependencies, and make changes there. Trying to contribute to the library from inside the vendor/ folder of your project is not going to work.

Also, without doing that, you can't run the tests, linter, etc, which means that you can't fully check that your changes are valid before you commit them. There's more details on how to contribute in the contributor guide.

@jlevers
Copy link
Owner

jlevers commented Aug 5, 2024

I tested this myself. Merging.

@jlevers jlevers merged commit a4f7d6d into jlevers:main Aug 5, 2024
2 checks passed
@simonsolutions simonsolutions deleted the patch-4 branch August 5, 2024 21:34
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