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

Removes defaultProperties method from various commands. Closes #6432 #6441

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SmitaNachan
Copy link
Contributor

Removes defaultProperties method from various commands. Closes #6432

@milanholemans
Copy link
Contributor

Thanks, we'll try to review it ASAP!

@Adam-it Adam-it added the hacktoberfest-accepted Accept for hacktoberfest, will merge later label Oct 29, 2024
@Adam-it
Copy link
Member

Adam-it commented Oct 29, 2024

@SmitaNachan I added the hacktoberfest-accepted label to this PR which means that this PR will count as done for the Hacktoberfest event. So if you participate in this event it will get you unblocked and it allows us to merge this PR later when we catch up 👍
Thanks for your support and awesome contribution 👏 You Rock 🤩

@Adam-it Adam-it self-assigned this Nov 14, 2024
Copy link
Member

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

@SmitaNachan awesome work 👏👏👏
I noticed that we missed also booking business get command which also has default properties but shouldn't. Do you think we could include this one as well so that we clear everything in one go

Also, unfortunately besides correcting the command and tests we will need to also update the command docs. In the docs we have the Response section in which we show the output of the command. If we now removed the default properties from those commands the output will not get trimmed and we now need to include the additional properties in the text response as well

@Adam-it Adam-it marked this pull request as draft November 20, 2024 22:07
@SmitaNachan
Copy link
Contributor Author

Hello @Adam-it
Thank you for the review comments. I will work on it.
Can you please guide how can I rebase to main? Appreciate if you can provide any Git commands for the same.

@Adam-it
Copy link
Member

Adam-it commented Nov 25, 2024

Hello @Adam-it Thank you for the review comments. I will work on it. Can you please guide how can I rebase to main? Appreciate if you can provide any Git commands for the same.

@SmitaNachan Sorry for the late reply 🙏. I saw your comment but I was totally full with everything.

Sure.

  • first you need to align main branch on your fork with main from this repo. You may do that using GitHub UI by going to your forked repo and clicking on the update button
    image
  • then open your forked repo that you have locally and make sure you are on the main branch. You may switch to main branch using the git switch main command
  • then run standard pull git pull so that you will download all changes from your forked repo main branch locally. Now your local version of main branch will be aligned with current state on this repo
  • next switch to you branch you are working on using git switch issue-6432
  • no do the rebase using git rebase main
  • if during rebase you will see some conflicts please solve them manually (I usually use VS Code for that) and then once all the conflicts are sort out don't commit them but run the git rebase --continue command which will continue with the rebase.
  • once the rebase is done you will need to push your updated branch to your repo. Since it will be different then what you have locally you will need to use the force option like this git push --force

If you will have any kind of problem just ping me up however you want.
You Rock 🤩

@Adam-it
Copy link
Member

Adam-it commented Nov 26, 2024

@pnp/cli-for-microsoft-365-maintainers due to my short brake I am unassigning myself from this one so that it may be processed by someone else

@Adam-it Adam-it removed their assignment Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accept for hacktoberfest, will merge later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove defining default properties from commands that are not returning a list
3 participants