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

feat(scoop-prefix): remove unused imports and functions. Load when required #4493

Closed

Conversation

pratikpc
Copy link
Contributor

@pratikpc pratikpc commented Nov 3, 2021

Unused imports removed:-

  1. manifest.ps1
  2. buckets.ps1

Unused function removed:-

  1. reset_aliases:- We do not use aliases here.

Load when required

  1. help needs to be loaded only when app is not specified
  2. core needs to be loaded only when app is specified

Gains:-

  1. Improved speed.

sinloss and others added 21 commits June 8, 2020 10:16
fix(tests): Force pester v4 (#4040)
The issue is the same as for #2952, workaround until the proper
solution is implemented. The code is also directly copied from there.

Fixes #4310
Fix #4000, handle arch-specific env_add_path
…quired

Unused imports removed:-
1. manifest.ps1
2. buckets.ps1

Unused function removed:-
1. reset_aliases:- We do not use aliases here.

Load when required
1. help needs to be loaded only when app is not specified
2. core needs to be loaded only when app is specified

Gains:-
1. Improved speed.
@rashil2000
Copy link
Member

Retarget to develop branch. Master is for release.

@pratikpc
Copy link
Contributor Author

pratikpc commented Nov 3, 2021

@rashil2000 are you sure?

Develop is behind master. It's 2nd most recent commit is from Oct 2020.

It's most recent commit from a few days back, which was from a PR opened in 2019.

To me something like this looks a bit weird.

Edit:- If this is the case, we should at least rebase develop on top of master. And stop new commits to master except via a split from develop.

@rashil2000
Copy link
Member

@niheaven would be able to suggest more on this.

@niheaven
Copy link
Member

niheaven commented Nov 4, 2021

@pratikpc Except for #4013 #4342 #4492, other merged PRs in master are only for text works. The recent commit (#3518) in develop is on the main develop line of scoop itself.

There are many feathers under development and they all should be committed into develop and tested by advanced users (incl. maintainers), only critical hotfixes that may affect normal users should be merged directly into master and synchronized to develop.

These commits that ahead develop now could be merged into develop after new commits in master and develop are all carefully tested, and then we can sync the branches and merge new PRs into develop and test them again.

@pratikpc pratikpc changed the base branch from master to develop November 4, 2021 11:52
@pratikpc pratikpc closed this Nov 4, 2021
@pratikpc pratikpc deleted the scoop-prefix-remove-unused-import-function-manifests branch November 4, 2021 11:52
@pratikpc
Copy link
Contributor Author

pratikpc commented Nov 4, 2021

That makes sense.

Although we really should first rebase develop on master so that we may not end up with future conflicts.

Also created a new PR

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.

9 participants