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

Report with links #63

Open
dukaev opened this issue Feb 22, 2019 · 8 comments
Open

Report with links #63

dukaev opened this issue Feb 22, 2019 · 8 comments

Comments

@dukaev
Copy link
Contributor

dukaev commented Feb 22, 2019

Report with links will be more informative.

lib/code.rb:9 Use attr_reader for reading ivars [https://github.com/DamirSvrtan/fasterer/docs/attr_reader_vs_ivars.md]
@DamirSvrtan
Copy link
Owner

it might make sense to point these to the fast ruby repo as they are the source of truth for those issues.

@DamirSvrtan
Copy link
Owner

@dukaev would you wanna take a stab at this? If not, i'd close it.

@dukaev
Copy link
Contributor Author

dukaev commented Feb 5, 2020

@DamirSvrtan yes. I'll do it within a week.

@dukaev
Copy link
Contributor Author

dukaev commented Feb 7, 2020

In the first version, it will be possible to do it quickly.
The second version allows configured output without links.

    EXPLANATIONS = {
      first: 'some warn [some_url.com]',
      second: {
        info: 'some warn',
        link: 'some_url.com'
      }
    }

@DamirSvrtan what do you think is the most suitable choice?

@DamirSvrtan
Copy link
Owner

Hi @dukaev 👋

I am sorry, I am not completely clear on your suggestion.

Is your first suggestion to do it like this:

    EXPLANATIONS = {
      first_offense: 'some warn [some_url.com]',
      second_offense: 'some other worn [some_other_url.com]'
    }

And the second one like this:

    EXPLANATIONS = {
      first_offense: {
        info: 'some_warn',
        link: 'some_url.com'
      },
      second_offense: {
        info: 'some other warn',
        link: 'some_other_url.com'
      }
    }

What do you think is the benefit of the first approach and what of the second one?

Would you link out all the reports to https://github.com/JuanitoFatas/fast-ruby ?

@dukaev
Copy link
Contributor Author

dukaev commented Feb 9, 2020

Hello @DamirSvrtan 🙂

In the first approach, first_offense: 'some warn [some_url.com]':
➕ No need to edit the logic of output. Only edit EXPLANATIONS.

In the second first_offense: { info: 'some_warn', link: 'some_url.com' }:
➕ Allows to costomize output with links / without links.

Yes. Links to JuanitoFatas/fast-ruby. Like https://github.com/JuanitoFatas/fast-ruby#hash-vs-openstruct-creation-code

@DamirSvrtan
Copy link
Owner

Hey Dukaev, let's go with the second solution (or something similar) where it's easy to grab the key of the offense or the link of the offense without additional parsing.

@dukaev
Copy link
Contributor Author

dukaev commented Mar 6, 2020

👋 @DamirSvrtan. Added PR #78

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants