news icon indicating copy to clipboard operation
news copied to clipboard

Clean up indentation + linting rules / Move all Components to Typescript

Open devlinjunker opened this issue 2 years ago • 1 comments

PR to Cleanup Linting rules + Use Typescript in all Components so far

  • follows https://github.com/nextcloud/news/pull/1831
  • indentation: use tabs rather than spaces
  • verify indentation in vue html templates
  • remove node/no-unpublished-import rule because no more vue-property-decorator library
  • Move all components use Vue.extend({ ...
  • Add types/ directory with .vue files containing type definitions used throughout app
  • no linting errors

For clarity, you can see this broken down into 2 steps here:

  • https://github.com/devlinjunker/news/pull/33
  • https://github.com/devlinjunker/news/pull/34

Up Next:

  • Add Unit Tests to run with npm run test
  • Add Vuex Store (see https://github.com/nextcloud/news/commit/58a196b548e8bf25896ee7db159dbecf88724635 for preview)
  • Add Proper Routing (see https://github.com/devlinjunker/news/compare/vue-version...devlinjunker:vue-routes for preview)

devlinjunker avatar Sep 16 '22 03:09 devlinjunker

I'd prefer if this was split into two reviews here too.

SMillerDev avatar Sep 16 '22 06:09 SMillerDev

I moved the branch pointer back 2 commits so it doesn't include the Component to typescript changes.

This PR is now just for cleaning up indentation and linting and I have created a new PR for moving the component files to typescript: https://github.com/nextcloud/news/pull/1922

devlinjunker avatar Sep 30 '22 02:09 devlinjunker

Why move to tabs by the way? Because all the other code in this repo uses spaces.

SMillerDev avatar Sep 30 '22 05:09 SMillerDev

@SMillerDev I was just following the Vue style conventions rather than add a rule to overwrite them. If you think it makes sense to keep the consistency in the entire repo then I can re-add the:

rules: {
        'vue/html-indent': 'off',
        indent: ['error', 4],
    },

to the .eslintrc file and run the lint again

devlinjunker avatar Sep 30 '22 06:09 devlinjunker

Updated the .editorconfig and those remaining spaces in the vue html @anoymouserver

devlinjunker avatar Oct 03 '22 03:10 devlinjunker

after this is merged, we can move on to https://github.com/nextcloud/news/pull/1922

and I am currently working on adding unit tests for the existing components. See https://github.com/devlinjunker/news/pull/56 for the WIP changes so far

devlinjunker avatar Oct 03 '22 03:10 devlinjunker