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

Project restructure #22

Merged
merged 10 commits into from Jun 25, 2019
Merged

Project restructure #22

merged 10 commits into from Jun 25, 2019

Conversation

rwky
Copy link

@rwky rwky commented Jun 15, 2019

This includes all lint fixes from @brettz9 and some restructuring so all passport-next projects and work from the same base project.

I've made https://github.com/passport-next/skel which acts as a skeleton template for all passport-next projects this is the first one to use it, this means if we want to say change a github template we do it in one repo and simply pull from that repo instead of having to edit each file individually on each repo, it supports jinja2 templates to make the files such as README.md etc

The eslint config has been forked from https://github.com/brettz9/eslint-config-ash-nazg to https://github.com/passport-next/eslint-config-passport-next it's almost identical at the moment, this just means that our config is under the org and we can pull in @brettz9 changes when we want.

Checklist

  • I have read the CONTRIBUTING guidelines.
  • I have added test cases which verify the correct operation of this feature or patch.
  • I have added documentation pertaining to this feature or patch.
  • The automated test suite ($ make test) executes successfully.
  • The automated code linting ($ npm run-script lint) executes successfully.

brettz9 and others added 7 commits May 27, 2019 11:04
    variables in more nested scope
- Linting: Remove jshint
- Linting: Apply overrides to add mocha-specific config
- Linting: Add recommended fields to `package.json`
- Linting: Remove unused disable directives
- Git/npm/Linting: ignore `var`
- Fix: Point to correct path in Makefile `view-cov`
    `authenticate.js` middleware when switching to strict mode with errors
    upon setting a property on a boolean )
- npm: No need for full path within package.json scripts
- remark lint default preferred styling
…e require-jsdoc for tests; use new preference for "object" over "Object"

- npm: Update devDeps
* Merged in https://github.com/passport-next/skel
* Updated project according to template
* Lint fixes
@coveralls
Copy link

coveralls commented Jun 15, 2019

Pull Request Test Coverage Report for Build 96

  • 157 of 159 (98.74%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 96.296%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/sessionmanager.js 17 19 89.47%
Totals Coverage Status
Change from base Build 71: -0.2%
Covered Lines: 336
Relevant Lines: 338

💛 - Coveralls

@rwky rwky requested a review from a team June 15, 2019 19:08
@rwky
Copy link
Author

rwky commented Jun 15, 2019

@brettz9 can you check this over and let me know if it looks ok? Any other @passport-next/developers want to chime in feel free!

@rwky rwky mentioned this pull request Jun 15, 2019
5 tasks
@brettz9
Copy link
Member

brettz9 commented Jun 16, 2019

This is exciting! Glad to see such a forward-looking approach... Have got a cold now, but intend to add some comments/questions...

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.npmignore Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
Copy link
Member

@brettz9 brettz9 left a comment

Choose a reason for hiding this comment

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

As mentioned, this looks great. I've made a few comments and requests. I could make the changes myself, but some I think involve a choice so let me know where to go from here.

@rwky
Copy link
Author

rwky commented Jun 16, 2019

👍 thanks for looking at that lot, I'll make some tweaks in a bit! Hope you feel better soon.

* Removed comma-dangle config since it's included in the config
* Updated .npmignore and .gitignore
* Removed licenses in favour of licence from package.json
* Cleaned up lint commands
* Removed deprecated test-cov and view-cov commands (TODO replace these)
@rwky
Copy link
Author

rwky commented Jun 18, 2019

@brettz9 ok check out out now

@rwky rwky requested a review from brettz9 June 18, 2019 17:08
templates/.gitignore.j2 Outdated Show resolved Hide resolved
.npmignore Outdated Show resolved Hide resolved
.npmignore Show resolved Hide resolved
@rwky
Copy link
Author

rwky commented Jun 25, 2019

OK fixed those typos, @brettz9 check again!

@brettz9
Copy link
Member

brettz9 commented Jun 25, 2019

All looks good to me!

@rwky rwky merged commit e056ba7 into master Jun 25, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants