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

Better Contribution flow #331

Open
6 tasks
pvinis opened this issue Dec 24, 2019 · 0 comments
Open
6 tasks

Better Contribution flow #331

pvinis opened this issue Dec 24, 2019 · 0 comments

Comments

@pvinis
Copy link

pvinis commented Dec 24, 2019

I tried adding an action. Here is what I had difficulties with, maybe it could be a point of possible improvement.

  • npm test is ran on every commit
    This causes the commits to take forever to happen. The tests should be ran ideally on the PR, not per commit.

  • interfaces/WF and interfaces/WF/WFWorkflowActionParameters.ts need some docs and coordination.
    I had trouble understanding if I need to reuse stuff in there, if I should add more, how to name them, where to put them. I think these two need some kind of rules to know when we need to add them and how to name them.

  • tslint autofix?
    I got some complaints for things that tslint could actually fix for me. Maybe a script in the package.json for autofixing would be useful. Also an editorconfig file for the spacing would be useful, as I have mine with tabs by default so I had to fix the spacing on the new file I made.

  • don't use default exports
    With export default:

// comment.ts
const comment = () => {}

export default comment

// index.ts
import comment from './comment'
export {
  comment,
}

With export:

// comment.ts
export const comment = () => {}

// index.ts
export { comment } from './comment'

Without default, we get fewer lines, less verbosity, and now the editor can autoimport by just typing the name of the function we want to use.

  • Inconsistent types with , and ;
    I think I saw some files using type A = { bla: boolean, foo: string } and some using interface B { bla: boolean; foo: string }. It would be nice to be consistent. I think tslint has a rule for this.

  • | in types
    I saw types like:

type A =
  'foo'
  | 'bla'
  | 'blu'

A better way to write this is:

type A =
  | 'foo'
  | 'bla'
  | 'blu'

It's easier to read, understand, and easier to run a sort func, like in vim.

I hope some of these make sense. These are just my suggestions, take for here what you like. I'm happy to discuss more about any of these. Thanks. :)

@pvinis pvinis mentioned this issue Dec 24, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant