Improve tree-shaking

Hi there,
I’ve had a quick glance at code and could make a PR, but feels better to ask maintainers first.
There is non-standard (yet widely supported) sideEffect field which, if present in package.json, allows build tools (example from Webpack docs) to better handle tree-shaking.
So, the code (at least of universal-cookie package) seems to be side effect free, but I’d like to get confirmation from you. If you haven’t ever dealt with sideEffects field, in this case we speak about side effects on module evaluation (i.e. import or require).
I tried to cover essential parts, but if further clarification is needed, feel free to ask me.

Yes I'm sure we can add this. Good idea!

Done with v4.0.3


✨ Modify ClusterExtension to use Helm under the hood to apply contents into the cluster

Description
ClusterExtension reconciler needs to perform the following actions:
[See this doc for more detail if possible.]

Based on the cluster Extension spec, get the list of bundles from the catalog and perform resolution.

The resolution here need not use dep.py, it can be simple filtering and sorting as done in extension controller currently.

Ref: Extension controller code
Resolution: Extension controller resolution
The resolution step involves fetching the installed version. Since there are no APIs (like BundleDeployment or App) present in clusters that reference the installed bundle, we need to investigate on listing the installed helm charts or use helm secrets. This also means that when the helm chart is created, we need to store metadata that uniquely identifies an extension.

Unpack the contents of the resolved bundle. For this iteration we can cover only registryV1 bundles from an image source.

Unpacker interface reference: Unpacker interface
Image unpacker: Image image unpacker
Instead of the BundleDeployment API we need to pass in the cluster extension directly.

Conversion of registryV1 to plain bundle.

Conversion logic: Convert registryV1

Creating a Helm chart from the resources present in plain bundle.

Plain bundle provisioner

Installing helm chart onto the cluster.

Helm chart deployer
We need to be able to inject metadata into the chart that will be required for resolution as mentioned in 1.(iii).

Currently, we run a post render hook, that adds labels on the objects created by the chart: Post render hook details. Will need to figure out on how to store this info in the helm secret or on the helm chart directory.

Reviewer Checklist

[ ] API Go Documentation
[x] Tests: Unit Tests (and E2E Tests, if appropriate)
[x] Comprehensive Commit Messages
[x] Links to related GitHub Issue(s)

@tmshort thanks for deconflicting main.go 👍

/retest required

/test ?

/retest

FYI for reviewers: I've merged a new commit to the branch which re-enables all e2e tests; they should be passing now. If you like, you can view the standalone diff here.

So, after a discussion, we decided to hopefully make it easier on all, to reduce churn on this PR.
We'd like to not make many changes to this PR unless absolutely necessary.
We will be creating follow up PRs for specific issues (e.g. errors fixes #878)

The only part remaining to be fixed in this PR are the unit tests - #874 is in progress to handle the same.

Codecov Report
Attention: Patch coverage is 65.65217% with 158 lines in your changes are missing coverage. Please review.

Project coverage is 76.08%. Comparing base (7727f8f) to head (b993945).

Files
Patch %
Lines

...nternal/controllers/clusterextension_controller.go
64.88%
82 Missing and 23 partials :warning:

internal/handler/registry.go
62.26%
10 Missing and 10 partials :warning:

internal/controllers/clusterextension_status.go
45.71%
19 Missing :warning:

cmd/manager/main.go
75.43%
9 Missing and 5 partials :warning:

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #846      +/-   ##
==========================================
- Coverage   83.18%   76.08%   -7.10%
==========================================
  Files          15       18       +3
  Lines         874     1154     +280
==========================================
+ Hits          727      878     +151
- Misses        102      198      +96
- Partials       45       78      +33

Flag
Coverage Δ

e2e
56.49% <61.73%> (-5.75%)
:arrow_down:

unit
51.18% <21.55%> (-23.76%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

@everettraven @m1kola @acornett21 PTAL
We've rebased the PR and updated the unit tests, and will be going forward with this. Other significant changes to be done have been noted in various issues.


Add genre aggregation

This should return the top 10 genre labels in doc count descending order.
Request:
?aggregations=genre

Response:
{
  "results": [...],
  "aggregations": {
    "type": "AggregationSet",
    "genre": {
      "type": "Aggregation",
      "buckets": [
        {
          "type": "AggregationBucket",
          "genre": {
            "type": "Genre",
            "label": "Painting"
          },
          "count": 100
        },
        ...
      ]
    }
  }
}

https://github.com/wellcometrust/catalogue/pull/194


nixos: nginx: setting forceSSL and listen can be silently weird

Describe the bug
Under services.nginx.virtualHosts.<name>; Setting a listen config that has no non-ssl listen and forceSSL generates a server { block in the final nginx config with no listen parameter, which nginx happily throws away with no warning.
Steps To Reproduce
nix build github:shelvacu/nixpkgs-nginx-bug#nixosConfigurations.default.config.system.build.toplevel && cat result/etc/nginx/nginx.conf
See that the first server { block is missing a listen clause
Expected behavior
Either add an assert so this fails to build, or automatically add a listen clause if one isn't present
I'm not 100% sure which is the proper solution why this is an issue and not a PR
Notify maintainers
@fpletz @ajs124 @raitobezarius

Add a :+1: reaction to issues you find important.

Check this variant:
diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix
index 93b1a3fdfadd..7a149538fbef 100644
--- a/nixos/modules/services/web-servers/nginx/default.nix
+++ b/nixos/modules/services/web-servers/nginx/default.nix
@@ -375,6 +375,7 @@ let
       in ''
         ${optionalString vhost.forceSSL ''
           server {
+            ${concatMapStringsSep "\n" listenString hostListen}
             ${concatMapStringsSep "\n" listenString redirectListen}

             server_name ${vhost.serverName} ${concatStringsSep " " vhost.serverAliases};

Hmm, no, this variant will not be correct.
The correct variant is to add a port without ssl support:
  listen = [
    { addr = "<IP_ADDRESS>"; port = 80; ssl = false; }
    { addr = "<IP_ADDRESS>"; port = 443; ssl = true; }
  ];


sidecar file fusion implementation

Hi guys
Just refactored and merged my branch sidecar_file into my master. This means that my master now holds an up to date version of both fusion and nuke sidecar_file implementations. All details are on the commit messages. In short, I implemented an error instead of trying to wing the rest of the code by faking empty dicts/tables when the manifest cannot be decoded.
Keep me posted!
Cheers
Cedric

Great, thank you. I'll merge this in and do a beta 3 very soon.
For release notes:

Added Fusion regex pattern matching
Support for sidecar manifests
Fixed updating issue when connecting fuse to different cryptomatte types
Performance improvements

Also, how do you feel about removing the "beta" tags from the fusion implementation (after the 1.2 beta?)

Hi Jonah
The regex pattern matching is not present in this branch. I will refactor my pattern branch upon this merge.
Fully agree on release notes and if removing the beta tag is fine for you, then it is for me.
I will keep you posted on the regex implementation, still battling with some Fusion behavior on the clamping side.
Cheers
Cedric

Oh ok, I'll remove that from the notes. I thought I saw that in the commit message for d965ceb but it must be referring to something else.

Oh hold on a second, you're right, when switching branches I must have comitted from the wrong one. Give me a second to rectify this.

Hold on a second, you are right. When switching branches I must have comitted from the regex one. Give me a second to rectify this.

Ah, ok.
If it's ready (enough) to go for a beta, feel free to leave it in, this is a beta release after all.

Ah no sorry, I triple checked and I removed it in later commits, that is when I split my code into two branches. So sorry about that, green light it is!
PS: It is indeed ready for beta release, but I am a little picky on that side.
Cheers
Cedric

No problem. I've merged it locally into my dev branch, and this PR will automatically be closed when the master branch becomes that. Thanks!

Alright. Always a pleasure and thank you!
Cheers
Cedric


[lit-next] Unify Prettier dependency to the root

Unify Prettier dependency to the root.
Also format all files in all packages.
I split the exec command (npm run format) in the last commit in case we want to delete it and run it at another moment.

Config moved to a .prettierrc.json file!

@aomarks merged with master and ready to review!


1.13 player disconnect when other players join

When players join, player using 1.13 get disconnected.
https://pastebin.com/FcWgMZJC

[13:41:03 WARN]: Caused by: io.netty.handler.codec.DecoderException: The
received string length is longer than maximum allowed (33 > 20)
[13:41:03 WARN]:        at net.minecraft.server.v1_8_R3.PacketDataSeriali
zer.c(PacketDataSerializer.java:219)
[13:41:03 WARN]:        at net.minecraft.server.v1_8_R3.PacketPlayInCusto

Try updating bungeecord, they recently reduced the length of their legacy message channels, beyond that, you have a bungeecord plugin that is sending messages which a channel name that is basically too large

<> could u explain that a little bit more. I doesnt really understand what u mean

Should be fixed by updating BungeeCord see above.
Plugin channels in 1.13 are in a different format which leads to them needing to be converted.


Enhance bake index template actions buttons with Icons

Whats about actions as Icons, just a suggestion.

Coverage remained the same at 90.65% when pulling 0bbd362664609af476e861f4a060522546139bfb on ayman-alkom:master into d085bd6e7446599a5bcd790e6f09dd1c8a749d0f on FriendsOfCake:master.

If the icons have a title attribute so tooltips are possible that would be great IMO.
Then :+1: from me.

I agree with @dereuromark.

That 's a great idea, I tested the title attribute, and the tool-tip didn't triggered automatically, we have to add a piece of code like this:
$(function () {
  $('[data-toggle="tooltip"]').tooltip()
})

I appended that code to the script block at the end of index.ctp

I dont think you need to add that snippet. People can do that on their own (or use any other JS lib). but the title attribute should at least be filled :) And so should the alt tag to be "correct".

sorry for these unnecessarily commits, the final case is without alt tag.
any way I think that we don't have to test our code over the browsers because it's already tested by the bootstrap itself.

Please squash the commits into a single commit.

I have done @ADmad , I am new to contributing but it is going well :+1:

@ayman-alkom Indeed! :+1:

Thanks all! ;)


Add versionless Cask for Droplr.app

Hi Vito,
Yeah, it's official. We'll put it up on the website once it's approved. Just let me know if you need anything else from us.
Cheers!

@biasedbit could you please squash your commits?
Thank you for confirming, @levinunnink. Please post here where on the website the link is, after you’ve done it.

could you please squash your commits?

I will, as soon as I get confirmation this is good to go, as stated in the message of the last commit.
PS: I appreciate the link but I've been using git for a while now, I know my way around.

I will, as soon as I get confirmation this is good to go

It is.

PS: I appreciate the link but I've been using git for a while now, I know my way around.

TextExpander snippet; I just cps. That’s why I always include a link with that request.

Done.

Please post here where on the website the link is, after you’ve done it.

This is a specific direct download URL for the cask and it's already up.

This is a specific direct download URL for the cask and it's already up.

That’s not what I meant. To merge this cask, there needs to be something on the actual website (webpage) that links to this specific url, or we can’t merge it (reasoning). Alternatively, you can provide a non-shortened url (i.e. one that starts with droplr.com instead of d.pr).
Without either of those the cask cannot be accepted, as there’s no way for users to know it is trustworthy (this conversation isn’t enough, a user needs to be able to audit a cask for what it is, not have to search through issues to find something).
There’s still the third option of treating this as a walled build and submitting to caskroom/unofficial, but that’s not a great solution.

Also see https://github.com/caskroom/homebrew-cask/pull/13683, where this conversation happened before.

We can do that, Vítor. It'll just take a bit.

Just created https://droplr.com/homebrew. Does that meet the requirements?

Just created https://droplr.com/homebrew. Does that meet the requirements?

Sure does.

Updated, tested, audited and rubocop'd.

Thank you for the changes and the fast response time. Merged.


Investigate why App8Test is very rarely failing

The App8Test sometimes fails with
[ERROR] Failures:
[ERROR]   App8Test>AbstractOpenApiGeneratorWebMvcIntTest.getOpenApiAsJson:17->AbstractOpenApiGeneratorWebMvcBaseIntTest.assertResponseBodyMatchesOpenApiJson:23 paths./simple/get-response-entity-instant.get.responses.200
Expected: content
     but none found
 ; paths./simple/get-response-entity-instant.get.responses.200
Expected: description
     but none found
 ; paths./simple/get-response-entity-instant.get.responses.200
Unexpected: $ref

 Actual JSON: {"openapi":"3.0.1","info":{"title":"API for App8","version":"unknown"},"paths":{"/simple/get-instant":{"get":{"operationId":"getMappingReturnInstant","responses":{"200":{"$ref":"#/components/responses/200"}}}},"/simple/get-instant-2/{pathParam}":{"get":{"operationId":"getMappingReturnInstantWithParameter","parameters":[{"name":"inputParameter1","in":"query","required":false,"schema":{"$ref":"#/components/schemas/inputParameter"}}],"responses":{"200":{"$ref":"#/components/responses/200"}}}},"/simple/get-response-entity-instant":{"get":{"operationId":"getResponseEntityOfInstant","responses":{"200":{"$ref":"#/components/responses/200"}}}}},"components":{"responses":{"200":{"description":"Default response","content":{"*/*":{"schema":{"$ref":"#/components/schemas/string_date-time"}}}}},"schemas":{"string_date-time":{"type":"string","format":"date-time"},"inputParameter":{"type":"string","format":"date-time"},"responseEntitySchema":{"type":"string","format":"date-time"}}}}

This happens when run in IntelliJ (all tests) and with mvn package. It was never observed when this test is run standalone (in IntelliJ). Running the same command again usually makes this error go away.

The referenced commit 9cc7974 may not close it, so re-open it again

As this hasn't happened for quite some time, I'll close that issue.


Suggestions for icons

I'm not a big fan of icons in general. Here are two (somewhat conflicting!) suggestions:

Add an option to suppress the icons in the 'Outline' pane.
These icons obscure the path, which imo is much more important.
True, these icons disappear when focus is outside the 'Outline' are, but an option to dispense with them would be welcome.

Add an icon/menu item to go to the LeoJS: Welcome pane.
This would complement the standard "gear" icon.
This suggestion is less important imo.

@edreamleo Thanks for those suggestion!
I'll add option settings, like I did for hover icons on the outline, shown below, to have the user select which ones he wants to have above the outline pane, if any, and I'll also add this concept for the icons above the body pane editor.  😄

@boltex You're welcome :-)

Outline-pane icons are now customizable in devel branch. It the leojs-settings webview offers most outline structure-editing related command icons.
Since the body pane is very limited in icon real-estate space, I'll leave the 'prev/next', 'extract, extract names' and other body pane icons as they are, since they are related to the body pane textual content.


Update gen_exported.cpp

Better envelope implementation

Thanks for the update.
I was thinking on the best way to handle custom changes, and there is just no way to do it in code that can handle all the possible cases of units, port properties, naming and other details.
So I think the best is just to allow these kind of plugins to ship with their own custom ttl file, that was hand-modified. So no need for extra work converting those changes into code just so they would be generated again..
Will set this up in a moment and ping you on the file needing tweaking.


Add constraints and belongs_to to additional_rights

The additional_rights table is an associative table.
Add relevant constraints and conventional Rails declarations.
Signed-off-by: David A. Wheeler<EMAIL_ADDRESS>

Other than being horrified by your Rubocop violations, LGTM.

New version coming up.  Perhaps the horror will dissipate :-).

Codecov Report

Merging #780 into master will decrease coverage by 91.87%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #780       +/-   ##
==========================================
- Coverage     100%   8.12%   -91.88%
==========================================
  Files          44      44
  Lines        1386    2905     +1519
==========================================
- Hits         1386     236     -1150
- Misses          0    2669     +2669

Impacted Files
Coverage Δ

app/models/project.rb
55.62% <100%> (-44.38%)
:arrow_down:

app/models/user.rb
50.76% <100%> (-49.24%)
:arrow_down:

app/models/additional_right.rb
100% <100%> (ø)
:arrow_up:

app/controllers/project_stats_controller.rb
0% <0%> (-100%)
:arrow_down:

app/controllers/application_controller.rb
0% <0%> (-100%)
:arrow_down:

app/models/badge.rb
0% <0%> (-100%)
:arrow_down:

app/lib/repo_files_examine_detective.rb
0% <0%> (-100%)
:arrow_down:

app/lib/floss_license_detective.rb
0% <0%> (-100%)
:arrow_down:

app/controllers/users_controller.rb
0% <0%> (-100%)
:arrow_down:

app/mailers/application_mailer.rb
0% <0%> (-100%)
:arrow_down:

... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 058c6c8...b26fbe4. Read the comment docs.


[EXT] Collections - Vector

https://docs.rs/near-sdk/3.1.0/near_sdk/collections/struct.Vector.html
For each of the following functions, add an example code snippet:

pub fn len
pub fn new
pub fn clear
pub fn push
pub fn extend
pub fn get

Examples should be simple, easy-to-follow, and useful to make onboarding and learning Rust SDK as straightforward as possible. See array primitive for examples of code snippet examples: https://doc.rust-lang.org/nightly/core/primitive.array.html), https://doc.rust-lang.org/src/std/collections/hash/map.rs.html#784-790 for examples in code.

Note that for all or most of the collection types, these are all legacy ones that will eventually be replaced by the near_sdk::store equivalents, so not sure if it's worth putting resources into.
The one exception to the new collections being documented with examples is actually the new Vector https://github.com/near/near-sdk-rs/blob/master/near-sdk/src/store/vec/mod.rs which could be improved.
Also, note that docs can be pulled from https://doc.rust-lang.org/std/vec/struct.Vec.html for the most part (esp for the new version because the API matches more closely than near_sdk::collections::Vector

I don't really get why the new Vector doesn't use u64 anymore but u32, if you plan on having more than u32::MAX items you cannot do this with this change. I guess your option is only UnorderedMap/LookupMap


app won't connect to DB

mvn package
cf push
Results in:
This application has no explicit mapping for /error, so you are seeing this as a fallback.
Mon May 01 14:35:55 UTC 2017
There was an unexpected error (type=Internal Server Error, status=500).
Failed to authenticate to database [CloudFoundry_8qq9og8g_53o2fv6c], username = [CloudFoundry_8qq9og8g_53o2fv6c_406ioqtj], password = [E******************************A]

@choman I've update the app to use the latest versions of Spring Boot, Spring Cloud Connectors, etc. Please update, re-build, and re-push. I also added a README that describes how to create and bind service instances to the app.
If you still have problems, please comment here and include the details of any services you have bound to the application and what Cloud Foundry environment you are deploying to.

@scottfrederick Based on the updates, all seems to work find.  README was a big help, thanks for throwing it together.  It saved me writing up another issue ;) - cheers