DamirSvrtan/fasterer

Report with links

dukaev opened this issue ยท 8 comments

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]

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

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

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

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?

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 ?

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

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.

๐Ÿ‘‹ @DamirSvrtan. Added PR #78