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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated configurations from Config #2630

Closed
AdmiringWorm opened this issue Mar 8, 2022 · 2 comments · Fixed by #2632
Closed

Remove deprecated configurations from Config #2630

AdmiringWorm opened this issue Mar 8, 2022 · 2 comments · Fixed by #2632
Assignees
@gep13
Labels
5 - Released Breaking Change
Milestone
2.0.0

Comments

@AdmiringWorm
Copy link
Member

The root elements cacheLocation and commandExecutionTimoutSeconds are deprecated as these are now handled in a different way instead together with other configuration values.
These two are scheduled to be removed in v1 of Chocolatey.

https://github.com/chocolatey/choco/blob/develop/src/chocolatey/infrastructure.app/configuration/ConfigFileSettings.cs#L30-L36
@AdmiringWorm AdmiringWorm added this to the 1.0.0 milestone Mar 8, 2022
@AdmiringWorm AdmiringWorm added the ADD AUTO TESTS Things that typically go to Test-Kitchen - once completed, IN REGRESSION TEST SUITE label added label Mar 8, 2022
@gep13 gep13 self-assigned this Mar 8, 2022
gep13 added a commit to gep13/choco that referenced this issue Mar 8, 2022
@gep13
(chocolatey#2630) Remove properties from ConfigFileSettings
47dbc4a 
These properties had previously been marked as Obsolete, and since we
are releasing the 1.0.0 version of Chocolatey, the decision has been
taken to remove these properties completely.

There were some usages of these properties, in the Equals and
GetHashCode methods for the class, but their removal isn't believed to
cause any issues.

A new OfEach method was added to the HashCode class to handle the
scenario where the first property passed in to generate the overall
hashcode for the class was of type IEnumerable.
@gep13 gep13 mentioned this issue Mar 8, 2022
Merged
9 tasks
@AdmiringWorm AdmiringWorm modified the milestones: 1.0.0, 2.0.0 Mar 10, 2022
@AdmiringWorm
Copy link
Member Author

This is bumped back to v2.0.0 instead. Removal of these will break the compatibility of some versions of other products we support, as such until we stop supporting these versions we can not remove these configuration values.

@AdmiringWorm AdmiringWorm added Blocked - External Requires Upstream Change Requires changes to a different location once issue is fixed or implemented labels Mar 10, 2022
gep13 added a commit to gep13/choco that referenced this issue Mar 17, 2023
@gep13
(chocolatey#2630) Remove properties from ConfigFileSettings
da19356 
These properties had previously been marked as Obsolete, and since we
are releasing the 2.0.0 version of Chocolatey, the decision has been
taken to remove these properties completely.

There were some usages of these properties, in the Equals and
GetHashCode methods for the class, but their removal isn't believed to
cause any issues.

In addition, we have moved away from using the custom HashCode class, in
favour of the new HashCode class that has been created in the .NET
Framework.  We didn't have access to this class before, since we were
still targetting .NET Framework 4.0.
gep13 added a commit that referenced this issue Mar 17, 2023
@gep13
Merge pull request #2632 from gep13/2630-remove-deprecated-properties…
4b65eca 
…-from-configfilesettings

(#2630) Remove properties from ConfigFileSettings
@gep13 gep13 added 4 - Done and removed 3 - Review Blocked - External Requires Upstream Change Requires changes to a different location once issue is fixed or implemented ADD AUTO TESTS Things that typically go to Test-Kitchen - once completed, IN REGRESSION TEST SUITE label added labels Mar 17, 2023
@choco-bot
Copy link

🎉 This issue has been resolved in version 2.0.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

gep13 added a commit to gep13/choco that referenced this issue Jun 22, 2023
@gep13
(chocolatey#2630) Re-instate setting of config properties
f6d797e 
Previously, in this commit:

chocolatey@da19356#diff-cb6a0471e41268b22a928bd57a59d51b70b7024e9beb30e89a330e193a089eba

The usage of the top level CacheLocation and
CommandExecutionTimeoutSeconds values had been removed, since these top
level properties within the chocolatey.config had been replaced with
values contained within the config section of the chocolatey.config
file.

However, the changes in that commit were too aggressive, and removed
the call to the set_config_item (which has subsequently been renamed to
SetConfigItem).  The method call does the work of taking any value that
is defined in the chocolatey.config for a given property name, and
adding it to the ChocolateyConfiguration instance.  When this method
call was removed, it stopped setting the property value on the
instance, and as a result, values that had been configured in the
chocolatey.config file were ignored.  They were still in play when the
configuration was passed in via a command line option, but not when
defining them in the chocolatey.config file.

The removal of this call to the set_config_item method also explains
why it was necessary to apply this bug fix in Chocolatey GUI:

chocolatey/ChocolateyGUI#1003

The method call also had the effect of setting the description on the
configuration value, which would have meant that Chocolatey GUI
wouldn't have thrown a null reference exception.  The change in
Chocolatey GUI is still valid though, as there are times when a config
value can have a missing description, so it makes sense to leave that
fix in place.
@gep13 gep13 mentioned this issue Jun 22, 2023
Merged
10 tasks
gep13 added a commit to gep13/choco that referenced this issue Jun 23, 2023
@gep13
(chocolatey#2630) Re-instate setting of config properties
318ee93 
Previously, in this commit:

chocolatey@da19356#diff-cb6a0471e41268b22a928bd57a59d51b70b7024e9beb30e89a330e193a089eba

The usage of the top level CacheLocation and
CommandExecutionTimeoutSeconds values had been removed, since these top
level properties within the chocolatey.config had been replaced with
values contained within the config section of the chocolatey.config
file.

However, the changes in that commit were too aggressive, and removed
the call to the set_config_item (which has subsequently been renamed to
SetConfigItem).  The method call does the work of taking any value that
is defined in the chocolatey.config for a given property name, and
adding it to the ChocolateyConfiguration instance.  When this method
call was removed, it stopped setting the property value on the
instance, and as a result, values that had been configured in the
chocolatey.config file were ignored.  They were still in play when the
configuration was passed in via a command line option, but not when
defining them in the chocolatey.config file.

The removal of this call to the set_config_item method also explains
why it was necessary to apply this bug fix in Chocolatey GUI:

chocolatey/ChocolateyGUI#1003

The method call also had the effect of setting the description on the
configuration value, which would have meant that Chocolatey GUI
wouldn't have thrown a null reference exception.  The change in
Chocolatey GUI is still valid though, as there are times when a config
value can have a missing description, so it makes sense to leave that
fix in place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gep13 gep13

Labels
5 - Released Breaking Change
Projects
None yet
Milestone
2.0.0
Development

Successfully merging a pull request may close this issue.

(#2630) Remove properties from ConfigFileSettings gep13/choco
3 participants
@gep13 @AdmiringWorm @choco-bot