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

Deprecate the overwrite option from github workflow add command #5776

Closed
Adam-it opened this issue Jan 15, 2024 · 6 comments
Closed

Deprecate the overwrite option from github workflow add command #5776

Adam-it opened this issue Jan 15, 2024 · 6 comments

Comments

@Adam-it
Copy link
Contributor

Adam-it commented Jan 15, 2024

CI/CD pipline is rather made to continuously redeploy and overwrite existing package with new one.
We should deprecate overwrite option from the command and make overwrite default behavior.

Then in v8 we should remove it
related issue #5765

@Adam-it
Copy link
Contributor Author

Adam-it commented Jan 26, 2024

on it

@Adam-it Adam-it self-assigned this Jan 26, 2024
@Adam-it
Copy link
Contributor Author

Adam-it commented Jan 26, 2024

@waldekmastykarz, @martinlingstuyl just having some second thoughts on this. Shouldn't we only mark the overwrite options as deprecated (add info in docs and add a warning when someone will be using it).
If we also make the change to make overwrite as default won't this be a change in the current behavior which is rather considered a breaking change thing 🤔?
Or do we consider this a bug?

@martinlingstuyl
Copy link
Contributor

Hmmm, I see what you mean. This option should be removed in v8, but to mark it deprecated right now seems to signal the wrong thing. In the new version we'll set the overwrite behavior as default and remove the option. In the current version we should just leave it in (of course) and not mark it deprecated, I guess...

@Adam-it
Copy link
Contributor Author

Adam-it commented Jan 27, 2024

exactly.
on one hand we will mark it as deprecated so someone may think it is not recommended to use this option
but on the other hand the intention is actually to use it always and we want to make it as default but since it will be a change in the behavior we may not do that just yet but with v8 🤦‍♂️
it's just so confusing
WhatHuhGIF

I totally agree with what you proposed @martinlingstuyl, leave it as it is now and just remove the option and change the behavior for v8.

@pnp/cli-for-microsoft-365-maintainers any additional feedback on this one?

@waldekmastykarz
Copy link
Member

Since the actual change that we want to do is to change the default behavior, what if we:

  • not deprecate the overwrite option now
  • in v8 make overwrite the default behavior
  • in v8 remove overwrite
  • right now, if you don't use overwrite, we show a warning saying that this behavior is deprecated and will be equal to overwrite in v8

Thoughts?

@Adam-it
Copy link
Contributor Author

Adam-it commented Jan 28, 2024

yep, that will work as well. Let's proceed this way

Adam-it added a commit to Adam-it/cli-microsoft365 that referenced this issue Jan 29, 2024
Adam-it added a commit to Adam-it/cli-microsoft365 that referenced this issue Jan 29, 2024
Adam-it added a commit to Adam-it/cli-microsoft365 that referenced this issue Feb 24, 2024
@waldekmastykarz waldekmastykarz added this to the v7.6 milestone Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants