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

Add Ruby config file option to installer #1327

Merged
merged 3 commits into from
Nov 11, 2024
Merged

Add Ruby config file option to installer #1327

merged 3 commits into from
Nov 11, 2024

Conversation

tombruijn
Copy link
Member

@tombruijn tombruijn commented Nov 5, 2024

Based on #1324

Rename matchers in installer spec

Differentiate between YAML install method and others in the matcher names so it's easier to add ones for the Ruby config file later.

Add Ruby config file option to installer

To replace the YAML config file people need to know they can configure the gem with a Ruby config file too. Add this option to the installer so people can choose this config option when installing AppSignal in their application.

I've made it the first listed method as it's the most flexible configuration method. The YAML file option has the "legacy" label next to it now to discourage people from using it.

Differentiate between YAML install method and others in the matcher
names so it's easier to add ones for the Ruby config file later.
To replace the YAML config file people need to know they can configure
the gem with a Ruby config file too. Add this option to the installer so
people can choose this config option when installing AppSignal in their
application.

I've made it the first listed method as it's the most flexible
configuration method. The YAML file option has the "legacy" label next
to it now to discourage people from using it.
@tombruijn tombruijn marked this pull request as ready for review November 5, 2024 10:49
Copy link
Contributor

@unflxw unflxw left a comment

Choose a reason for hiding this comment

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

The YAML file option has the "legacy" label next to it now to discourage people from using it.

Could/should we remove it entirely? 🤔 The installer is mostly used by new AppSignal users, I don't think they'll miss it.

Either way, now that the YAML is considered legacy, are there things that need to be changed regarding how we document configuration options? Perhaps the order of the code snippets and sections in the configuration overview page should be changed to put .rb first.

resources/appsignal.rb.erb Outdated Show resolved Hide resolved
@backlog-helper

This comment has been minimized.

1 similar comment
@backlog-helper
Copy link


This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

@tombruijn
Copy link
Member Author

The YAML file option has the "legacy" label next to it now to discourage people from using it.

Could/should we remove it entirely? 🤔 The installer is mostly used by new AppSignal users, I don't think they'll miss it.

I don't want to shock and confuse existing customers that are used to configure with the YAML file and don't have the time to adapt to the new thing right away. So I don't want to introduce a new thing and take out the old way in the same version. It's legacy now and we can choose to remove it in a version before the next major version.

Either way, now that the YAML is considered legacy, are there things that need to be changed regarding how we document configuration options? Perhaps the order of the code snippets and sections in the configuration overview page should be changed to put .rb first.

Yes, we should update the docs to make the appsignal.rb file method first class.

@tombruijn tombruijn changed the base branch from config-rb-file to main November 11, 2024 16:43
Remove the list of errors that isn't needed to include all.

Link to the docs of the options listed.
@tombruijn tombruijn merged commit f5381a0 into main Nov 11, 2024
127 checks passed
@tombruijn tombruijn deleted the config-rb-install branch November 11, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants