Settings do not stick after exiting PowerToys

Microsoft PowerToys version
0.59.1
Running as admin

[ ] Yes

Area(s) with issue?
Settings
Steps to reproduce

Launch PowerToys
Change any setting
Exit PowerToys
Launch PowerToys again
Previous change is reverted

✔ Expected Behavior
Changing a setting under any user context/app running mode will retain that setting after exiting and re-launching.
❌ Actual Behavior
Settings are not retained after exiting and re-launching. No setting for any module is retained, including General: Always run as administrator, Run at startup, and (if running in Admin Mode) Download updates automatically.
Other Software
No response

I uninstalled PT and deleted c:\users\<username>\appdata\local\Microsoft\PowerToys for user and admin accounts. After reinstalling, no changes stick regardless of user mode versus admin mode. Additionally, when running in Admin Mode, Video Conference Mute displays a banner "You need to run as administrator to modify these settings".
Is there another location that stores settings and should be deleted after uninstalling to ensure a fully clean installation?

The run PowerToys as another user scenario is not well supported. Does it save if you don't run as admin, though? Changes should still be saved in %localappdata%\Microsoft\PowerToys for your normal user. /needinfo

Sorry, accidentally closed and don't know how to re-open without commenting again

Problem is that PowerToys.Settings.exe still runs in the context of currently logged in user after elevation with another account. More details are in #22659

Fix is merged, for more info check this issue https://github.com/microsoft/PowerToys/issues/22659

This has been addressed as part of the 0.66 release cycle. Thank you!


[FR] When we select a post and it is a reply, open the right threading pane, revealing the the replied to post

Rationale:
I find that I always do this:

See a toot that's a reply
Wonder what is the context
Select it
Right pane opens up
Press k to select it
Press k again, to see the replied to toot

Current:

Suggested:

Maybe have to make provisions if the original toot is too tall?

🤔 Hmm, but k is already taken for navigating to "next post"

I don’t understand. ‘k‘ moves up, so should be previous post. But don’t have to change the keys; my suggestion was to show the replied post at the top instead of the selected post. Provides more context.

@hboon ohh lol sorry mistake.
Okay, I just tried this on Twitter web, it works exactly as what you suggested. I'll look into this.

Haha, I thought I was first!
But just to be sure, when I visit https://twitter.com/pm/status/1628822358512500736 and click on the 2nd tweet "6 years, turns out to be a reasonable number." also by pm, it shows the tweet I click at the top:

not this (which I suggested):

It does makes sense to show the selected tweet at the top if the URL is going to be the tweet. But I think it's better to give it a URL that implies some kind of threading, then maybe scrolling to the replied to tweet would be more sensible.
Anyway, just a thought :)

I came here to ask for the same thing, and just want to add that when I click on a toot it's never just to see the same toot one more time ;) It's always to either like/boost/fav or to see the context. And in all cases just showing the previous toot in the sidebar could not hurt, IMO. Now I always have to move my mouse pointer all the way from the leftmost side to the rightmost side just to scroll up to do the thing I clicked on the toot to do anyway, and then move it all the way back again to keep scrolling my home feed. This gets tiring fast. That's why I'd appreciate it if at least the one toot above the one I click on was shown.
Otherwise I want to say thank you for making a truly beautiful Mastodon client! Well done! :)


升级最新依赖 并修复eslint报错

General

✏ Mark the necessary items without changing the structure of the PR template.

[ ] Pull request template structure not broken

Type

ℹ What types of changes does your code introduce?

👉 Put an x in the boxes that apply

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Checklist

ℹ Check all checkboxes - this will indicate that you have done everything in accordance with the rules in CONTRIBUTING.

👉 Put an x in the boxes that apply.

[ ] My code follows the style guidelines of this project
[ ] Is the code format correct
[ ] Is the git submission information standard?
[ ] My code follows the style guidelines of this project
[ ] I have performed a self-review of my own code
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[ ] My changes generate no new warnings
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] New and existing unit tests pass locally with my changes
[ ] Any dependent changes have been merged and published in downstream modules

vue升级3.3.4的话，unplugin-vue-define-options可以移除掉了

能解决这个问题吗https://github.com/vbenjs/vue-vben-admin/issues/2898

能解决这个问题吗#2898

解决了

什么时候能合入呢？

这条pr 依赖升级会引发的严重问题， 复现： clone新仓库，依赖安装完后执行lint 指令，会报如下错误

stylelint 傻掉了 找不到命令。


HTTPS bumps

I want to view bumps over HTTPS pls.
https://letsencrypt.org/

:package:

@qpfiffer important https://community.letsencrypt.org/t/acme-v2-and-wildcard-certificate-support-is-live/55579

o hell ye

Le 14 mars 2018 à 14:20, dx<EMAIL_ADDRESS>a écrit :
@qpfiffer important https://community.letsencrypt.org/t/acme-v2-and-wildcard-certificate-support-is-live/55579
—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub
https://github.com/qpfiffer/shithouse.tv/issues/21#issuecomment-373937913,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADrvWrmpxqzSrjyH2gM-bVOHQ_bmW830ks5tfUgcgaJpZM4HgOma
.


added alexeystar.com

added alexeystar.com;
edited formatting and data-lang for id=slewis

Hey Alexey!
Very nice website :) Welcome to the webring. It'd be better I think if the webring was simply the icon, you can embed it in the page itself if you don't want to add an extra url call. But either way, it makes me to add it to the webring! Loved the posts on your post with the different pangrams in Georgian.

Hey @neauoire! Thanks for adding me, I'm excited to join the webring! I'm trying to keep primary pages as lightwegith as possible, hence even 250B matter to me and I'd prefer a text link to an image, if possible. It could even be rendered like this: {ⅩⅩⅠⅠⅤⅤ}, with curly brackets linked to the prev/next site in the ring 🤓


Deploying is difficult, especially for projects created locally first

I was asked by someone at MS to log my feedback. My colleagues and I were part of an internal hackathon. Our idea was to use the bot framework to create a Facebook Messenger bot. I (strongly) proposed using the bot framework.
On the day of the competition it was easy enough to go File > New Project and create a bot. We could also debug locally using the emulator. Great!
But then we tried to deploy it and what a nightmare that turned out to be. I thought it was as easy as right click > publish. Nope... then I found the above article.
What was bizarre about the above process is that I had to start with the same project from Azure. I couldn't use my existing project. I also had to go through the convoluted process and use Kudu (the only option really).
I spent ages trying to bolt in our code into the echo bot sample project and then to get it working. Just downloading the echo bot and being able to re-deploy it was time consuming.
Only after an all-nighter and a very early morning did I manage to get a basic app actually working ahead of our presentation, with about an hour to spare.
I'm surprised how difficult it is to create and deploy a bot, especially as it's v4.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 2cfa1653-6ab2-730b-5479-81a05b9bb18e
Version Independent ID: a7c8085e-3b7a-711e-308c-7061176c0a5c
Content: Deploy your bot - Bot Service
Content Source: articles/bot-builder-deploy-az-cli.md
Service: bot-service
Product: bot-framework
GitHub Login: @ivorb
Microsoft Alias: v-ivorb

There is on-going work to make deployment less painful. Unfortunately, it also means the current process is a moving target. PR 1311 includes an update to this article, and should go live this week.

@JonathanFingold the PR 1311 line above leads to a 404 page
https://github.com/MicrosoftDocs/bot-docs-pr/pull/1311

The PR was merged back in February. The deployment articles have had a number of revisions since then, and the dev team is still working out the kinks in the process, so this will continue to evolve for a month or two.

@JonathanFingold thank you for your reply, please do you have changeset number for the merge?

@pkaushik23 , is this what you're looking for?
https://github.com/MicrosoftDocs/bot-docs-pr/commit/50276807c5be30ea842eb97b246d207dc5b06b14

sorry, but the link points to 404.
[image: image.png]
On Mon, Apr 1, 2019 at 9:24 PM Jonathan Fingold<EMAIL_ADDRESS>wrote:

@pkaushik23 https://github.com/pkaushik23 , is this what you're looking
for?
MicrosoftDocs/bot-docs-pr@5027680
https://github.com/MicrosoftDocs/bot-docs-pr/commit/50276807c5be30ea842eb97b246d207dc5b06b14
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MicrosoftDocs/bot-docs/issues/865#issuecomment-478637412,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABxF-hDJG8usN7-jMPlpQHA0Aef16J9Nks5vciuqgaJpZM4bCEFW
.

--
Thanks and regards
Prerak Kaushik

Sorry, bot-docs-pr is the private repo. This is probably the commit you're after, through there have been other changes to the article since then: https://github.com/MicrosoftDocs/bot-docs/commit/05ddade244874b7d6e2fc91745131b99cc58b0d6#diff-2f92b7bf10ac615faea324908b550101


Simplify Mvc.Testing usage

The current Mvc.Testing API has a few quirks that make it less usable / understandable:

WebApplicationFactory<TEntryPoint> exposes a ClientOptions property that

is only used when calling CreateClient() i.e. it's ignored when calling CreateDefaultClient(). This is documented but not at all intuitive
duplicates the CreateClient(WebApplicationFactoryClientOptions) feature. Why do both approaches exist?

The WebApplicationFactory<TEntryPoint>.CreateDefaultClient(Uri baseAddress, ...) makes it simple to specify the base address of the created HttpClient. No similar WebApplicationFactory<TEntryPoint>.CreateClient(...) overload exists; users must either instantiate a WebApplicationFactoryClientOptions instance or change ``WebApplicationFactory.ClientOptions.BaseAddress` to do the same thing.

@danroth27, would be great to hear your opinion about this

I think these are by design, but @javiercn can say for sure.

@dougbu what is your proposal here?

@javiercn I suggest

remove the ClientOptions property because CreateClient(WebApplicationFactoryClientOptions) is clearer

perhaps leave a static _clientOptions or _defaultClientOptions field for CreateClient()'s use

add a CreateClient(Uri baseAddress) overload

remove the ClientOptions property because CreateClient(WebApplicationFactoryClientOptions) is clearer

The most common use case for this class is to call CreateClient(). ClientOptions is there so that you can configure how the clients are created by default for all your tests.
For tests where you need to customize something (like avoid autoredirects, or use a different base Uri) you can pass in the client options directly to get the client you want instead of the default one configured for the factory.
For tests where you need complete control (very few) you have CreateDefaultClient which will allow you to explicitly configure the list of handlers applied to your client (useful for scenarios that involve authentication for example).
As a convenience, in that method you get an overload to set the base address along with the list of handlers to apply to the client.
Testing in most real applications requires both, handling cookies and redirects automatically to make the tests bearable. Which is why CreateClient exists and why it's considered the main API.
CreateDefaultClient is there for the 5% scenarios where you need absolute control over the list of handlers+address applied to the request.

For tests where you need complete control (very few) …

This is very strange.  But, you need to switch to CreateDefaultClient(...) for "complete" control?

CreateDefaultClient is there for the 5% scenarios where you need absolute control over the list of handlers+address applied to the request.

Why is the "default" the one that doesn't work like a real client and is supposedly for the 5% case?

@dougbu Because I wanted a different name and @Eilon preferred default. Default in this context means "Empty/No additional behavior"

@javiercn if that's the case, then the following overloads would make more sense:

CreateClient(params DelegatingHandler[])
CreateClient(Uri, params DelegatingHandler[])
CreateClient(WebApplicationFactoryClientOptions)
CreateDefaultClient()
CreateDefaultClient(Uri)

Actually, do we have an example of anyone / anything except WebApplicationFactoryClientOptions using DelegatingHandlers? It's not clear why the handlers parameters are publicly exposed.

Hmm, #7638 and #7637 are interesting given a user's WebApplicationFactory<T> subclass is never returned from WithWebHostBuilder(...). See issue #7541 and the fix in #7542 / 51784bb2d6. Can't down-cast what WithWebHostBuilder(...) returns to a user type.
That is, neither PR composes well with other parts of the Mvc.Testing package. Maybe we should switch the #7541 fix to a protected virtual Clone() approach to enable these requested features end-to-end?

We would pass in a delegate in the same way we do for other methods for the webhostbuilder factory, so no need to switch the approach.

Pass delegates into what? WithWebHostBuilder(...) is not virtual.

Closing this issue as some of the aspects of the WebApplicationFactory class have been improved already. Also, we'll look forward for customer feedback going forward and make sure that subsequent feedback is address in future releases.


Make the StrictInArray sniff fixable.

Can this be auto-fixable though? What if there is code that relies on a form submission that sends ?foo=1 and the PHP you have does in_array( $_GET['foo'], array( 1, 2, 3 ) ). If this gets fixed, then this code will then break. Same goes for fixing == to ===. Auto-fixing these may break the logic.

If this gets fixed, then this code will then break, Same goes for fixing == to ===. Auto-fixing these may break the logic.

True, but that indicates an underlying issue with the comparison logic which should be addressed anyway. Loose type checks are the cause of lots of bugs.
If you don't believe me, come join me for the big "Why Equal doesn't Equal" Quiz some time if you get the chance ;-)
http://phpcheatsheets.com/ FTW :sunglasses:

@jrfnl I absolutely agree. But I think the underlying premise of phpcbf is that it can fix problems automatically without any impact on functionality. If we try to automatically fix loose type checks, there is a good chance the automatic fix would break the logic. So I think it cannot be a fixable error. I would not feel comfortable using phpcbf if I knew it could break my code.

Makes sense to me. Let's close this for now.


Add native support to docker scripts for passing Nvidia Graphics into Docker

This add support for simply created docker containers with nvidia support.
The creation of container should work in both cases, so that's the reason to add Nvidia supported configuration.

@mamueluth can we somehow reduce number of docker file to only one?
There are two important changes:

image name
L:31 has to be added to nvidia docker file. (This is maybe something that can be filtered out using SED. Or we just leave it as it shouldn't make any difference in the standard configuration. What do you think?)


Add dashboard loading indicator

CATEGORY
Choose one

[ ] Bug Fix
[x] Enhancement (new features, refinement)
[ ] Refactor
[ ] Add tests
[ ] Build / Development Environment
[ ] Documentation

SUMMARY
Adds a top-level dashboard loading indicator.
Today, we show loading indicators overlaying each chart on the dashboard, but we don't show a top-level indicator for the full dashboard. This is problematic because the loading indicators are sometimes easy to miss (e.g., loading is happening in other dashboard sub-tabs, or loading indicator on chart has low-contrast).
Additional Background:
I use Superset heavily (on the Airbnb Human Team), and have been working with non-engs that use these dashboards frequently. I keep getting feedback on different perf-related issues, and I feel like it's eroding their trust in the data.
I'm throwing up this "central" loading indicator to make it clearer that no more "loading" is taking place on the dashboard, so they can hopefully have more confidence that the numbers are "ready". (Before this, they would toggle dashboard filters, and the chart-by-chart loading indicators would cause them to feel unsure whether the data is ready to be interpreted.)
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Dashboard while loading
Dashboard after loading finishes

TEST PLAN
Followed CONTRIBUTING.md docs, set up local Superset instance and confirmed changes locally.
ADDITIONAL INFORMATION

[ ] Has associated issue:
[x] Changes UI
[ ] Requires DB Migration.
[ ] Confirm DB Migration upgrade and downgrade tested.
[ ] Introduces new feature or API
[ ] Removes existing feature or API

REVIEWERS
@williaster @john-bodley @michellethomas (unsure who to tag)

Codecov Report

Merging #143 into airbnb-master will increase coverage by <.01%.
The diff coverage is 100%.

@@                Coverage Diff                @@
##           airbnb-master     #143      +/-   ##
=================================================
+ Coverage          65.06%   65.06%   +<.01%
=================================================
  Files                427      427
  Lines              20680    20681       +1
  Branches            2313     2314       +1
=================================================
+ Hits               13456    13457       +1
  Misses              7100     7100
  Partials             124      124

Impacted Files
Coverage Δ

...uperset/assets/src/dashboard/components/Header.jsx
41.58% <100%> (+0.58%)
:arrow_up:

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 ce1b307...6d49ad6. Read the comment docs.

Hi @harrytruong, thanks for the PR! In general we only use this repo for cherrypicking fixes from open source; I'd recommend making this PR against apache/incubator-superset and then retagging us for review. Thanks!

Thanks for the heads up @etr2460! Will close and re-open against apache/incubator-superset


Add config option for JSON Serializer

Could there be a jsonSerializer() method on ConfigBuilder? So that I don't have to do the following, in case in the future the sender architecture changes.
.sender(
  new BufferedSender.Builder()
    .sender(
      new SyncSender.Builder()
        .accessToken(token)
        .jsonSerializer(jacksonSerializer)
        .build()
    )
    .build()
)

Good idea!


Question on status code 599

I was trying to work out why nothing printed when my service was down and I finally got it by listening for the fetchdataerror event, however when an ECONNREFUSED error is handled I wonder whether 503 Service Unavailable would be more appropriate rather than the non-standard 599 status code.

Happy to submit a pull request for this, if you agree.

In this instance, 599 is deliberately nonstandard — we've experienced an error internally to the client, and we want to make sure it's distinct from a server response.
If we were a proxy communicating via HTTP, I'd agree that 503 would be more appropriate, as we'd be signalling that we received an upstream connection error. But in this case I feel like the error should be distinct (unless you've got a compelling rationale to the contrary!) :)

That said, it's funny that you got a fetchdataerror and not a fetcherror in this case. Are you able to put together a small test case demonstrating the problem?

I may be wrong about which error event it was, there are a few ;) I'll check.

I understand your point of view, but from the wikipedia page on status codes it is not implied that 503 is only for downstream proxy services:

The server is currently unavailable (because it is overloaded or down for maintenance). Generally, this is a temporary state.

However I see how you would want to distinguish this error from others. I can easily switch 599 to 503 in my code anyhow, or maybe we should treat this as a completely separate error event for the implementation to handle?

Sorry my bad, it was actually fetchclienterror which I believe is correct semantically.

In this case, fetchclienterror always indicates that there was an error making the request, so you can safely handle the 599s in isolation using this event. If you scan the queue independent of the event, you can use the 599 to be able to determine that a particular queue item failed at the client rather than at the server.
Perhaps the 5XX series of errors is not the place to put this, since it's a client error, not a server error. We could change it to 499, or maybe invent a new 6XX error code, for instance.

Hmm, I'm not totally sure about that. If the server is down how is it a client error? Sure it's a client request error but the fault lies with the server.
Personally I would be really keen to stick with the existing standard HTTP response codes that people are familiar with ;)

Well I think it's important that it's clear from the status code that the error wasn't delivered via HTTP from the destination. If we used 503, say, while it might be semantically correct, we wouldn't be able to determine whether the server returned the 503 or whether we invented it because we couldn't get upstream (even if it's the server's fault.)
I'm inclined to use a non-standard code purely because it won't be confused with anything a server might return.

I see that point of view too, your call of course ;)

maybe invent a new 6XX error code

Out of all the options so far and given your feeling for a non-standard code for this scenario, I think maybe the above is the best option: 600.
I actually checked if 599 was a valid status code just to be sure before I raised this, if people see 600 for sure they know something is up!

I like your reasoning around the error code, @cgiffard, I agree it would be more confusing if we swapped it out for 503 (or any other valid status code).
But I don't have any objections to changing the status code to 600, it might make things clearer. It's easier to filter based on status anyway, so it probably isn't that big of a deal if we were to change the status code

@cgiffard decision on this one?

@fredrikekelund Let's go with 600, and make sure it's in the docs. I'm happy to do that if you're busy!

@fredrikekelund Thinking we should probably start assigning issues so we don't double up, haha.

@cgiffard cool! Yeah, good idea - I'll leave this one to you then :)

Nice! I'll fix it now. :)

Fixed in 88f8a08907446bde8584730d5a1105a6dbc18d35.
Queued up for an 0.8 release when ready!

Thanks for keeping the momentum going folks, I just upgraded to 0.7.0 without a hitch and will check this out when 0.8.0 is released.

Hah, it's mostly @fredrikekelund doing all the work! (I've blocked out today to try and catch up myself, haha.)
I've closed this issue because the code has been merged, and unless something horrendously untoward happens, it'll be released soon!


Fail loudly with an invalid token

Is there an endpoint we can use to check this?

I was thinking along the lines of not rescuing here. Either:

The user gives us a token, and the token is invalid. They've shown intent to use the token, but we're unable to do so and we should error.

The user gave us a repo, and the repo is invalid. They've shown intent to use a repo's metadata, but we're unable to do so, and should error.

Lol, @jekyllbot.


Block Library: Import from/Export to XML file

Overview
Replaced the 'Import Block' and 'Export Block' buttons with 'Import Block Library' and 'Export Block Library'. User uploads XML files that are parsed and stored into the block library. When they are done or are happy with their block library, they can export their block library as each block's xml separated by a new line.

This change is

@vicng @picklesrus @evd2014

Review status: 0 of 5 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion.

demos/blockfactoryv2/app_controller.js, line 66 [r1] (raw file):

      // Update block library controller with the new block library
      // storage.
      self.blockLibraryController.storage = blockLibStorage;

I think this is reaching a little to far into the controller.  The storage member should probably be private.  Sounds like what you might want is a method in the controller (something like importBlockLibrary or reloadBlockLibrary) that takes the blocklibrary name and xml as params.
Also, does importing wipe out the blocks currently in the library. Should it?

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 1 unresolved discussion.

demos/blockfactoryv2/app_controller.js, line 66 [r1] (raw file):
Previously, picklesrus wrote…
> I think this is reaching a little to far into the controller.  The storage member should probably be private.  Sounds like what you might want is a method in the controller (something like importBlockLibrary or reloadBlockLibrary) that takes the blocklibrary name and xml as params.
>
> Also, does importing wipe out the blocks currently in the library. Should it?
>

The reason I don't want the storage member to be private is so that the exporter may reference that same block library storage instance.
Importing blocks does wipe out the blocks currently in library. We talked about this on Friday, and agreed that this was probably the way to go. I am going to add some warning to allow the user to know that they should export the library before importing a different one.

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 3 unresolved discussions.

demos/blockfactoryv2/app_controller.js, line 40 [r2] (raw file):

  var self = this;
  var files = document.getElementById('files');
  // If the file list is empty, they user likely canceled in the dialog.

"the" instead of "they"

demos/blockfactoryv2/app_controller.js, line 56 [r2] (raw file):

        // Parse the file to get map of block type to xml text.
        blockXmlTextMap = self.formatBlockLibForImport_(fileContents);
      } catch (e) {

The file contents might be too much for an alert message. Can you just print the file name?

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 3 unresolved discussions.

demos/blockfactoryv2/app_controller.js, line 40 [r2] (raw file):
Previously, evd2014 (Emma Dauterman) wrote…
> "the" instead of "they"

Done.

demos/blockfactoryv2/app_controller.js, line 56 [r2] (raw file):
Previously, evd2014 (Emma Dauterman) wrote…
> The file contents might be too much for an alert message. Can you just print the file name?

Done.

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 4 unresolved discussions.

demos/blockfactoryv2/app_controller.js, line 66 [r1] (raw file):
Previously, quachtina96 (Tina Quach) wrote…
> The reason I don't want the storage member to be private is so that the exporter may reference that same block library storage instance.
>
> Importing blocks does wipe out the blocks currently in library. We talked about this on Friday, and agreed that this was probably the way to go. I am going to add some warning to allow the user to know that they should export the library before importing a different one.

+1 to Katelyn's comment. `storage` doesn't seem like a property that should be modified without a wrapper method.

demos/blockfactoryv2/app_controller.js, line 71 [r1] (raw file):

      // Update the exporter's block library storage.
      self.exporter.blockLibStorage = blockLibStorage;

Does the exporter really need a copy of block lib storage? I'd think it could exist independently.

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 4 unresolved discussions.

demos/blockfactoryv2/app_controller.js, line 66 [r1] (raw file):
Previously, vicng wrote…
> +1 to Katelyn's comment. `storage` doesn't seem like a property that should be modified without a wrapper method.

Done!

demos/blockfactoryv2/app_controller.js, line 71 [r1] (raw file):
Previously, vicng wrote…
> Does the exporter really need a copy of block lib storage? I'd think it could exist independently.

Yes it does so that the information that comes from the block library storage doesn't have to travel from the storage through the block lib controller through the app controller and down to the exporter.

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 4 unresolved discussions.

demos/blockfactoryv2/app_controller.js, line 71 [r1] (raw file):
Previously, quachtina96 (Tina Quach) wrote…
> Yes it does so that the information that comes from the block library storage doesn't have to travel from the storage through the block lib controller through the app controller and down to the exporter.

Okay. Let's keep an eye on this, especially workspace factory is integrated.  getters/setters are better than public members, but they can also be a code smell :)

Comments from Reviewable


Add secret label for filtering informers

Changes

Adds a label using the secret template for supporting filtering informers for the generated secrets
Part of the work here: https://github.com/knative-sandbox/net-istio/pull/920

Release Note

Secrets automatically generated due to certificate creation are labeled with a special label key to support proper filtering from K8s informers in components that consume them.

@dprotaso this is ready too.

/lgtm
/approve


Make the Cache::get() method return a Result instead of an Option.

Consider implementing a custom Result that contains 3 variants:

CacheHit - file was found in the cache.
CacheMiss - File was not found in cache, but was found in the filesystem.
Err - File was not found in either the cache or the fs.

An ok() method could resolve this back to the existing Option, or this Err case could be rolled into the existing CachedFile implementation and would redirect to the 404 handler when responding, meaning that get() would return just a plain CachedFile.

implemented this by rolling an error case into CachedFile.


Is it possible to add a global coordinate system to the corner

Is it possible to add a global coordinate system to the corner of the window, which rotates with the object ?
like this

Definitely a good idea - also a center dot / crosshairs would be helpful.


x/website: go.dev tends not to show up on Google search results, but tip.golang.org does

Go version
n/a
Output of go env in your module/workspace:
n/a

What did you do?
I performed Google searches for various documents that I know are hosted on go.dev. Example search queries:

golang doc comments
golang 1.23 release notes

What did you see happen?
The copy of the document on tip.golang.org was the first Google search result each time.
Here are the two examples:

(I noticed this on my personal Google account, but I took those two screenshots from a Chrome incognito window.)
What did you expect to see?
I expect the canonical version of these documents to be the first search result, or at least near the top.

golang doc comments:

https://tip.golang.org/doc/comment is the first result
https://go.dev/doc/comment isn't anywhere in the first two pages of search results

golang 1.23 release notes:

https://tip.golang.org/doc/go1.23 is the first result
https://go.dev/doc/go1.23 isn't anywhere in the first two pages of search results
Weirdly, the second search result is a go.dev URL, the "Go 1.23 is released" blog post at https://go.dev/blog/go1.23.

I guess for most projects I probably wouldn't file an SEO ticket, but you folks are mostly paid by Google, so I figure you ought to be able to sort it out :)

@golang/tools-team

I'm guessing this is because golang.org had historical momentum, and tip.golang.org doesn't redirect in the same way that golang.org does.
Perhaps we should just introduce tip.go.dev, and redirect tip.golang.org. I imagine then both would be downranked, but perhaps eventually go.dev links would make their way to the top.
(I know we work at Google, but I for one have very little understanding of how this works :))

Perhaps I shouldn't have focused so much on tip.golang.org. My main point with filing this issue is that there seems to be some SEO issue preventing the canonical versions of important go.dev pages from appearing anywhere near the top of Google search results in my recent tests.


Compiler error in folder Editor

Im getting compiler error

Hi, that's related to some weird bug on Assembly Definitions in Unity, i recommend cloning the repository and importing only the folders "Editor"/"Runtime"


chore(style): improve consistency

If approved, simply squash merge with the PR title. <3

Oh no!

You should've squashed it.

This way ^


fix: remove warnings for 3.11+

Fixes #540

thanks for this fix - those pre-commit bugs don't seem to be related to bugs or this change, so let's spot-check those in follow ups


#666 Improve IdentityComparer logic.

Implements #666.

@mafurman,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

:shipit: rebase, squash and commit.

Pushed directly to dev.


Shorter citation count format

Hi,
I just tried to use the MAS plugin to replace Google Scholar plugin to handle citation count in my Zotero library.
It works great and thank you for the effort to develop the plugin for the Zotero community.
However, is it possible to shorten the information in the Extra column?
For example, a record in the Extra column can be "ECC: 0000025". I need to make the column width large enough to see it has 25 citations, and it takes up space.
Could that be something as "00025", or even better, an option for us to set how many digits to show?
Best,
Jake

Hi Jake,
thanks for the feedback! The reason for the zeros is that all the citation entries have to have the same amount of digits to be sortable. If you use a lower amount of digits, entries with high citation counts can't be represented anymore.
There are two ways to fix this problem:
(1) Add an option to change the amount of digits or just automatically have the amount of digits be equivalent to the digits needed to represent the highest cited entry in your library. E.g. your highest cited entry has 122 citations therefore the Extra column for your 25 citation papers would look like this "ECC: 025". (or maybe "025 ECC" so you would need even less space)
(2) Add a column to represent the citation count and don't put it in the Extra column. There would be no need for the leading zeros anymore.
Implementing (1) should be pretty easy, and I should get around to it next week. I think (2) takes a little bit more time, but I want to eventually do that to also add other columns with more metadata information.
Best,
Tobi

I implemented (1) in b3e31c7e9ad21c0f5a90f3d45b1ce2ca27b416a6 and made a new release with it. Let me know if it works for you!

Just tried, it works perfectly, thanks!
Jake


Use django-redis instead of deprecated django-redis-cache

Pull Request Test Coverage Report for Build<PHONE_NUMBER>5
Details

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 0.0%

Totals

Change from base Build<PHONE_NUMBER>0:
0.0%

Covered Lines:
0

Relevant Lines:
0

💛  - Coveralls


Revert "Update HTTPS and Storage Explorer instructions. (#431)"

Reverts Azure/Azurite#435

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.:x: lubas569 sign nowYou have signed the CLA already but the status is still pending? Let us recheck it.

@XiaoningLiu
Would you please help to review the readme change?
There are many changes.

This PR doesn't make sense to revert readme expected changes.


Compilation error

I tried to compile this with mingw32-make, but I get the following errors:
undefined reference to `GetTickCount64'
undefined reference to `InitializeCriticalSectionEx'
recipe for target 'lib/libcurl.dll' failed
recipe for target 'cpr/opt/curl/lib/CMakeFiles/libcurl.dir/all' failed

Did you configure it beforehand somehow with CMake or it's GUI?
Undefined reference is a linker error, the linker can't find a matching function that your code specifies.
It's not a bug, it's user error on your part.


Linking ecto_autoslug_field

Hi, thanks for the awesome package.
Is there any chance to link ecto_autoslug_field from this package's readme since ecto_autoslug_field is built on top of the slugger?

Thank you for this hint, will add a reference in readme.

Done :D

Thanks!


Allow Symfony 4

See #3124

SF 4 is not published yet on packagist.org so it will not be used in the CI suit with the composer.json in your PR.

also, shall go for 2.2

"minimum-stability": "dev",

no
it doen't make Travis install 4.x: https://travis-ci.org/FriendsOfPHP/PHP-CS-Fixer/jobs/283234906#L550

I don't know how to force 4.x then

I removed the stability flag and tried to force the Symfony version in travis, I'm not sure if it's working tho

You can check if it works by looking at the output of Travis, for example:
https://travis-ci.org/FriendsOfPHP/PHP-CS-Fixer/jobs/284272226#L545
this show that for PHP 7.2 SF3 components are installed by composer. On PHP 7.1 it is the same.

please change target branch to 2.2 line

It looks like basing on 2.2 is causing a mess. I close this PR and will open a new one

This is because your branch includes your commits but also all commits from 2.6 that are not in 2.2. You need to rebase your branch onto 2.2 to get rid of these commits and keep only the ones you added.

See #3134


Scaleup not working when minReplicas: 0 and using PercentageRunnersBusy

Checks

[X] I've already read https://github.com/actions-runner-controller/actions-runner-controller/blob/master/TROUBLESHOOTING.md and I'm sure my issue is not covered in the troubleshooting guide.

Controller Version
0.26.0
Helm Chart Version
0.21.0
CertManager Version
1.9.1
Deployment Method
Helm
cert-manager installation
Cert manager installed following the guidelines on https://cert-manager.io/docs/installation/helm/#installing-with-helm from official source
Checks

[X] This isn't a question or user support case (For Q&A and community support, go to Discussions. It might also be a good idea to contract with any of contributors and maintainers if your business is so critical and therefore you need priority support
[X] I've read releasenotes before submitting this issue and I'm sure it's not due to any recently-introduced backward-incompatible changes
[X] My actions-runner-controller version (v0.x.y) does support the feature
[X] I've already upgraded ARC (including the CRDs, see charts/actions-runner-controller/docs/UPGRADING.md for details) to the latest and it didn't fix the issue

Resource Definitions
apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:
  name: actions-runner-deployment
  namespace: mynamespace
spec:
  template:
    spec:
      organization: myorg
---
apiVersion: actions.summerwind.dev/v1alpha1
kind: HorizontalRunnerAutoscaler
metadata:
  name: actions-runner-deployment-autoscaler
  namespace: mynamespace
spec:
  scaleTargetRef:
    name: actions-runner-deployment
  minReplicas: 0
  maxReplicas: 100
  metrics:
  - type: PercentageRunnersBusy
    scaleUpThreshold: '1'
    scaleDownThreshold: '1'
    ScaleUpAdjustment: '1'
    ScaleDownAdjustment: '1'
  scaleUpTriggers:
  - githubEvent:
      checkRun:
        types: ["created"]
        status: "queued"
    amount: 1
    duration: "10m"`

To Reproduce
Launch an action from a github repositoy, the scaleup never happens.

Describe the bug
Runner never scale from 0 to 1 when an action is triggered.
Describe the expected behavior
The runner scale to 1 when an action is triggered.
Whole Controller Logs
https://gist.github.com/rptxeni/f33e8461db88c8edb6bed2fb310da528

Whole Runner Pod Logs
NA

Additional Context
If minReplicas is set to 1, the action is executed on a runner and the scaleup/scaledown is working.
However, we must always have at least one runner.

Your logs suggest you're using the percentage busy sclaing metric, this metric doesn't support scaling from 0 as documented in the docs https://github.com/actions-runner-controller/actions-runner-controller/blob/master/docs/detailed-docs.md#autoscaling-tofrom-0


Prevent component from reloading when re-arranging

Hello and thanks for this really great project,
can you tell me if there is a way to not reload the panel-content when rearranging its position? In certain situations it does, in others not:
For example:
I have a Dockview component including one panel. I open another one with openPanel in the direction "right" - Only the new dockview reloads, which is as expected. When I now open another panel with openPanel in the direction "below" the new panel AND the one before (the second) reload.
I hope I described it understandable - let me know if not, I am happy for any help.
Best regards

Hey - sorry for the late reply.
By reloads do you mean the web page is repainting the content or something more specific to Reacts rendering? If possible an example could be very useful to see this in action. I was having a quick play in this sandbox but maybe you could adjust it to show your problem? https://codesandbox.io/s/inspiring-shadow-eoybz1?file=/src/App.tsx

I've had a similar problem in dock-spawn-ts (https://node-projects.github.io/dock-spawn-ts/)
We fixed this, by keeping the content of the panel always in the dome, and positioning it absolute to the position where the dockwindow is.

Hello, thanks for your replies!
I actually found out my problem is not related to dockview. I use the dockview to display multiple iframes - so when I investigated further by trying the same simple div - which is displaying its content after 5 seconds (setTimeout) - I get different results when using:

just the div
an iframe with srcDoc attribute
an iframe with src attribute

If it is just the div or an iframe with the srcDoc attribute it worked perfectly fine! But an iframe with the src attribute results in this:

I have one initial Panel
I add another panel to the right -> only the new Panel reloads (darkgrey background means it is reloading)
I add another panel below the second Panel -> second and third panel reload (this is my problem)

Thats how it looks:

With divs and iframe with srcDoc attribute it behaves like this (as expected):

So thanks for trying to help me here and again for the great dockview solution - I will investigate iframe behaviours instead :)
Best regards


support for extension for several bases

Any time need add one extension for many databases. But in postgresql::server::extension postgres extension name st as name of puppet resource. But puppet do not create many resources with one name.
I move extension name to parameter from resource name. Now we can set up many resource postgresql::server::extension for one extension name for many databases.

This looks like a duplicate of #637 ?

Yes. Sorry for duplicate...

No worries! Thank you for your contribution anyways!


Case change breaks builds?

Not sure what's started to happen, but overnight our builds have started to break:
can't load package: package github.com/TykTechnologies/tyk: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"
I wonder if it's the case change from S to s in the github username of the account. Even if we change the import path to use the lowercase version, dependencies that use the uppercase version that are not under our control seem to need updating too.
Vendoring the lib doesn;t seem to work either, as the log.WithFields seems to self referential, but maybe I was too hasty in dismissing vendoring:
./api.go:124: cannot use "github.com/TykTechnologies/tyk/vendor/github.com/Sirupsen/logrus".Fields literal (type "github.com/TykTechnologies/tyk/vendor/github.com/Sirupsen/logrus".Fields) as type "github.com/Sirupsen/logrus".Fields in argument to log.WithFields
Any tips appreciated.

Disregard the vendoring bit, I think that's our centralised logger screwing up.

I can't get gobgpd to build anymore either, but I'm having trouble tracking down why.
$ go get -v -u github.com/osrg/gobgp/gobgpd
github.com/osrg/gobgp (download)
package github.com/osrg/gobgp/gobgpd: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

The error doesn't say where the culprit file is, a grep through ALL my dependencies show that the only package that imports a lower case version in all my dependencies is Sirupsen/logrus
$ grep -r "github.com/sirupsen/logrus" ~/dev/local/go-workspaces/dependencies/
src/github.com/Sirupsen/logrus/alt_exit_test.go:	"github.com/sirupsen/logrus"
... snip ....
src/github.com/Sirupsen/logrus/README.md:  logrus_syslog "github.com/sirupsen/logrus/hooks/syslog"
src/github.com/Sirupsen/logrus/README.md:| [Syslog](https://github.com/sirupsen/logrus/blob/master/hooks/syslog/syslog.go) | Send errors to remote syslog server. Uses standard library `log/syslog` behind the scenes. |
src/github.com/Sirupsen/logrus/README.md:  log "github.com/sirupsen/logrus"

So looking at this, you have Sirupsen importing the lower case version..? I think you need to roll this back and come up with a better name change strategy because anything I try to build that imports your package breaks. I tried to cp -a Sirupsen to sirupsen .. apparently TIL[1], so I can't have them side by side.. I don't know how to fix this other than adding another go path with a lower case version just for your library. Anyways, everyone is going to run into this at some point I think it needs rolled back.
[1] http://apple.stackexchange.com/questions/22297/is-bash-in-osx-case-insensitive

@lonelycode Did you happen to find a fix for this? Everything I try results in breakage, least on OSX because I can't have both on the same file system. Changing my GOPATH still broke it. For now I am just checking out a older version so I can get up and running locally, but all my docker-compose / automation needs a work around if you happen to find one, thanks.

We can't afford to have something like this happen anymore, so we've vendored everything so we can control the namespace.
Since we use multiple logrus plugins (both from @sirupsen and from external contributors), basically:

Internal references use the lowercased version
external deps that have not been updated use the uppercase version

The two cannot live together at all, vendoring will not work, because external plugins (and internal ones) require the lowercased version as a type and you will either get a FGield type mismatch or you will get a case insensitivity issue.
So the only solution was to take it all in house :-/

Take a look at pull request 384 from yesterday for reference

This also broke us.  Can you revert PR 384 @sirupsen?

Sorry for the long reply...
Conventions are great - I'm a big believer in solid conventions and following them - but following conventions should be done before creating a dependency tree.
Changes like this have a significant impact downstream, especially if those projects use third-party libs/plugins that havent updated their import paths so the only solutions are to:

Wait for these third party Devs to update their code (or accept your PR - which still leaves you in the lurch)
Internalise everything
Work off of pinned revisions (a very unpalatable option)

We use 3 or 4 hooks, some provided by this repo and others external. And they all disagree as to what to reference, this change broke the build for about 5 of our projects and about 3 dependent internal libs.
Personally I prefer to not vendor because it means that stable, non-breaking updates make it into your build organically, and breaking changes are usually documented by library maintainers well enough to make for a painless transition - non-breaking backwards compatibility is pretty core to Go AFAIK, and it's a pattern to be followed, because it encourages responsibility on bahalf of the library maintainer.
But because logrus is a pluggable lib, it has an ecosystem that is dependent on its stability, and like all ecosystems those extensions only update at the behest of their maintainers - which is never a given in OSS.
That means changing the core dependency so it breaks the ecosystem should be done with great care, because now after this PR, the logrus ecosystem is broken and unusable until all hook developers, and formatters, and plugin builders update their code.
We've mitigated this upstream risk by forking and modifying all dependent projects -including logrus - because we can't wait for the ecosystem to self-correct, that was effort we could have expended elsewhere.
(Yes these should all be pull requests to the existing maintainers, and they will be - but to get to production waiting for a PR in an automated environment is not an option)
This is a great library, so please keep making it better and extending it, but please consider introducing major breaking changes more carefully.

Yes, this was due to https://github.com/sirupsen/logrus/pull/384.

please consider introducing major breaking changes more carefully

We did. These things are not taken lightly. Logrus favours stability at this point over features and re-design. On the other side of the coin, you have a large body of people who had issues with the previous one. I believe that this is the most sustainable path forward.
I apologize for the pain this has caused everyone in this threat. I understand that the last thing you want to have break your builds, is your logging library. I want to make sure you understand that Logrus favour stability and backwards compatibility over all else. I run production too, and I understand for 99% of Logrus users they use the standard API, community extensions and don't care much for a logging library failing their build.
Logrus is a foundational piece of infrastructure, and it should, and will, change minimally.
Logrus is in need of a healthy maintainer group, and if you think you can help us make better decisions, I'd love to have you join us and help @lonelycode.

Thanks for the response @siripsen, I'm sure the decision wasn't taken lightly. I's a great library (we love it), it was just very frustrating watching our builds fail overnight and then not finding a clean update path.
Happy to get involved - will start sending out those PRs :-)

For any Glide users affected by this change, the GitHub name change also poses a problem. See a workaround here: https://github.com/sirupsen/logrus/pull/384#issuecomment-264035988
The solution we went with for now was locking to an old Logrus version and making the repo URL explicit.

This still hasn't been rolled back, are you keeping this change? Every single persons build who uses your library is broken right now.. over a capital letter. If you truly felt it was imperative to have a all lowercase package name and the project is your #1 priority- why not move it into it's own organization with a new name all together to give people a chance to migrate.. github.com/logrus/logrus or something.
I think this should be reverted and reevaluated, there is not even a fix for this. I would need to hunt down every single dependency of every library.. and have them update in a single transaction across multiple organizations and repos .. all to change the casing of a letter to .. what? Keep your name inside the url? Seems petty.. just my opinion, it's your repo, name and code.

Are we sure that a revert is not going to cause even more pain at this point?
@stevvooe what do you think?

Given the backlash here I'd like to say I'm seriously considering reverting. I'm very surprised by the amount of people who don't have Logrus vendored, or in other words, how many people this is breaking for.

Is this really as simple as reverting that PR? Doesn't it mean I need to rename myself back to @Sirupsen as well?

@sirupsen Reverting the PR will not cause more pain, people who solved this problem by vendoring will still be vendored. People who can't vendor because they don't control the build system, repos, etc will be fixed. You don't need to revert your name since both Sirupsen and siruspen resolve properly, so only the pull request needs reverted.

On it.

Fixed in master.

I've added a public comment about this to the README should anyone come across this.
I apologize for this. I underestimated the impact this would have had. This was not acceptable, and not a justified thing to break backwards incompatibility for. If we wish to provide a lower-case name-change, we'll do so under a different name (e.g. under a logrus organization).

I'm keeping this issue open as some people may browse here looking for an open issue.

@sirupsen Classy, I respect your grace with this. This resolved the issue for all the automated builds that use go get on repos that don't vendor. On the local repo copy I still have, continued to work fine after go get -u. The only users that may have been affected would be anyone who was able to update their code since this morning, I imagine those users can quickly pivot back just the same. Thanks for fixing it for the rest of us.

We didn't have any build failures, since we use vendoring, but I was able to reproduce this earlier today with gobgp.
While I didn't have a build failure, I thought I'd show my line of troubleshooting. On my local box (OS X), where I use go get extensively, I updated by moving github.com/Sirupsen to githhub.com/sirupsen. You can try to clear out $GOPATH/pkg in case there are mixed case packages lingering. This works fine for shallow imports.
With larger projects, more is required. This is the build error I received with gobgp:
$ go build github.com/osrg/gobgp/gobgpd
can't load package: package github.com/osrg/gobgp/gobgpd: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus"

Nothing a little perl can't solve:
$ find ~/go/src/github.com/osrg/gobgp -type f -name '*.go' | xargs perl -i -pne 's|Sirupsen|sirupsen|'

After running that, the build works fine.
This is probably not a "production" solution, but neither is "go get".

@stevvooe https://github.com/osrg/gobgp/pull/1182
I believe now we don't have any problem on gobgp master w/o any modification because the package name has reverted a couple hours ago

@s2ugimot Thanks! I saw the PR, since I was going to upstream it. Too bad this was rolled back.
Does gobgp deploy production code built with go get?

@stevvooe

Does gobgp deploy production code built with go get?
Yes. But I found this problem when I was playing around with it on lab, not for building binary for production deploy. On production environment we build binaries within the docker container using go get and preserve it so that we can safely reproduce the same binary later on. Dependent package name change like this is one of the exact example which we have expected to happen. In this case it was reverted anyways...

preserve it so that we can safely reproduce the same binary later on

At least you can audit later if malicious code gets pulled down.
I'd highly recommend exploring vendoring for repeatable builds. It does seem clunky but is rock solid. You can still have a "side-build" that deletes the vendor directory to keep track of upstreams are breaking.

@stevvooe Thanks, I'll look into vendoring. And I think I also need to talk to the developers: https://github.com/osrg/gobgp/issues/963

@stevvooe there was plenty of ways to resolve it, but only one way that resolved itself. I'm not sure what your point is about "go get" and production, or "gobgp" production code being built with "go get". This isn't a production issue, no one here was claiming they had systems down.
The complaint was a needless interruption in everyones workflow. Maybe you don't, but I along with many other developers manage their many (or one) GOPATHS for local development using go get. I don't vendor every single command, binary, project, etc that I use day-to-day. Many of those projects don't vendor their own libraries.
With how fast the Go ecosystem moves and interesting new libraries are popping up I don't see how someone could ever discourage the use of go get, then to justify breaking many peoples workflows over a capital letter on their file system system.. just seems unusual to me.
@s2ugimot I think vendoring may be a good idea given the scale of GoBGP and the fact the binary commands (gobgp gobgpd) are widely used ad-hoc with go-get.

I side with @sirupsen's suggestion here https://github.com/sirupsen/logrus/pull/384#issuecomment-264333311, in were he suggests this to be continued in logrus/logrus.
This ensures support for those vendoring, or go get'ing, either upper or lower cases.
However - I feel a repo change shouldn't be purely on a naming and convention basis, but should have a larger driving factor.

vendor/github.com/Sirupsen/logrus/examples/hook/hook.go:12: cannot use airbrake.NewHook(123, "xyz", "development") (type *airbrake.airbrakeHook) as type "github.com/kcmerrill/crush/vendor/github.com/Sirupsen/logrus".Hook in argument to log.Hooks.Add: 	*airbrake.airbrakeHook does not implement "github.com/kcmerrill/crush/vendor/github.com/Sirupsen/logrus".Hook (wrong type for Fire method) 		have Fire(*"github.com/Sirupsen/logrus".Entry) error 		want Fire(*"github.com/kcmerrill/crush/vendor/github.com/Sirupsen/logrus".Entry) error
^^^ just for anybody who is googling(this is the error I was getting). Took me a while to find this thread(didn't realize readme was the problem I was experiencing).
@sirupsen thanks for all your work with logrus. ❤ it.

Reverted, so I'm going to close this.

@sirupsen Just noticed that your username is lowercase again when some glide commands started complaining about VCS not matching the remote. Is the change intentional?

I'm getting very confused with this. It seems the change is back for good (https://github.com/bshuster-repo/logrus-logstash-hook/issues/32#issuecomment-303195028), could you confirm please?

@gravis upper case works fine, but if you are using the examples it may force you to use lower-case—or if you are mixing dependencies that use lower-case and upper-case. We need to converge towards lower-case, because this is a mess no matter how you slice it.

Ok thanks. I have a bunch of hooks to update then, I hope users will understand.
I think I would have fallen in the same trap as you :(

Thank you for your understanding @gravis 😢 It's a regretful situation, but I hope as a result people update their dependencies and get the best out of it.

I think the pain point for this bug is just how subtle it is.. even more so on FS with case folding. It's unfortunate you didn't decide to move to a new repo, everyone could have used both versions side by side and upgraded painlessly. With aliases coming you could eventually nudge people further to the new repo with aliases. Oh well.

@cstockton I agree with you, and I am aware of that mistake.

@sirupsen My point was just that if you agreed with my comment here https://github.com/sirupsen/logrus/issues/451#issuecomment-264319311 then I'm confused why you would start pushing the breaking change months later. But it's your project, thanks for being polite regardless.

@sirupsen - I changed my gomfile and lowercased it, but still I am getting following error in both cases ( Capital and small ).
can't load package: package github.com/sirupsen/logrus/examples/hook: case-insensitive import collision: "github.com/Sirupsen/logrus" and "github.com/sirupsen/logrus". What should I do now.

Why are you importing the example hook?

My gomfile have  following line
gom github.com/sirupsen/logrus
But while doing a build it shows above error.

what do you think of this? https://github.com/meatballhat/negroni-logrus/issues/28#issuecomment-305216526

@gravis I'm concerned that if we start to use gopkg too many paths will collide, we already have two alternative paths.. this would give a third 😬 That would be the best thing long-term, absolutely—to give the freedom to iterate on the API.

Hmm, you're right. Ok, so let's move to sirupsen then.
Thanks!

This is confusing, what's the right way to do this now? In the readme it says:

Seeing weird case-sensitive problems? See this issue. This change has been reverted. I apologize for causing this. I greatly underestimated the impact this would have. Logrus strives for stability and backwards compatibility and failed to provide that.

but it also says:

The organization's name was changed to lower-case--and this will not be changed back. If you are getting import conflicts due to case sensitivity, please use the lower-case import: github.com/sirupsen/logrus. - Source

just wanted to let folks who're monitoring this issue know that golang/dep will soon have some more sophisticated reporting around this problem: golang/dep#1079. we can't actually solve it in dep - doing so requires rewriting import statements, which we strictly do not do - but we can guide the user to where the problem is, and to know which projects need issues filed against them to change from S -> s.

@sdboyer Any chance at getting this fixed in the upstream Go compiler? This case is particularly insidious in that they can't actually exist next to each other.

@stevvooe depends on what you mean by "fixed"? all the compiler can do is tell you that it's not allowed to have import paths that vary only by case - which it actually does: https://github.com/golang/dep/issues/433#issuecomment-317146614 is that what you meant?
i know i was certainly happy when a contributor pointed that out, as i'd assumed the compiler just didn't care, which meant that dep would be disallowing a case that the compiler allows. that would've caused a ruckus.

@sdboyer I think the problem here is that there is case insensitivity until there is not. It's been awhile since I've investigated, but, if I remember correctly, the type name ends up being case sensitive, causing the collision.
I don't think dep can do anything here. The compiler would probably have to flatten case for the package paths, but I am unclear on the specifics.

@sdboyer What I mean is that case-sensitivity is not enforced until the user can longer deal with the problem. What is the point in allowing the symbol golang.docker.io/docker/vendor/github.com/Sirupsen/logrus.Errorf if introducing golang.docker.io/docker/vendor/github.com/sirupsen/logrus.Errorf just causes a compiler error? These should be case-folded much earlier in the process.

@stevvooe ok, i'm a bit confused at this point, as it seems like you're trying to convince me of something that we're already doing. maybe we're just talking past each other somehow. because:

What is the point in allowing the symbol golang.docker.io/docker/vendor/github.com/Sirupsen/logrus.Errorf if introducing golang.docker.io/docker/vendor/github.com/sirupsen/logrus.Errorf just causes a compiler error?

yes. agreed. 💯. and disallowing that case is exactly what we're doing, because it causes a compiler error. i've said or alluded to as much in every one of my last three comments, and the links i've already provided add more detail.

@sdboyer My question has been asking about a whether or not this should be a fix in the Go toolchain (see https://github.com/sirupsen/logrus/issues/451#issuecomment-326649038). There is no point in making the dep tool, go get and GOPATH support case-sensitive import paths when the compiler explicitly disallows it.
Packages with the same name but different casing need to be treated as the same package. While golang/dep#1079, it is unrealistic to have the solver look for solutions that will select a variant of github.com/Sdboyer/deptest based on what is imported. In reality, I want this to use whatever package is canonically folded to github.com/sdboyer/deptest. There is no use case for having multiple instances, even of they all agree on the same import casing.

My question has been asking about a whether or not this should be a fix in the Go toolchain

yes, i saw that, but that doesn't make sense. the go toolchain doesn't do any kind of version selection right now. that's the problem we're fixing in dep. there is no conceivable way of "fixing" this in the go toolchain, apart from moving the responsibility for version selection into the go toolchain, which is the point of dep in the first place.

There is no point in making the dep tool, go get and GOPATH support case-sensitive import paths when the compiler explicitly disallows it.

i don't know why you think anyone is proposing this. i'm not. the PRs aren't. the PRs here REMOVE case sensitivity from (portions of) the import path, in a way that brings them into line with the compiler.

Packages with the same name but different casing need to be treated as the same package.

yes. agreed. and that's what the changes to dep in that PR are enforcing.

it is unrealistic

i'm not sure what this means. the PR literally works, so it's "realistic" in that sense. and it adds no new crazy logic in the solving algorithm - just one more dimension of satisfiability, wherein we verify that all projects active in the depgraph agree on a case variant for all project roots. now, that's not nothing - it's bumping the constant factor on an NP-hard search algorithm - but adding one more dimension doesn't just push us from "realistic" to "unrealistic."

In reality, I want this to use whatever package is canonically folded to github.com/sdboyer/deptest. There is no use case for having multiple instances, even of they all agree on the same import casing.

so, wait - you're saying you want the system (the compiler, dep, etc.) to enforce that import paths must be written in their case-folded form? if so, then OK, we do have a disagreement, at minimum because:

that would render large swathes of existing Go code invalid
it would require the compiler to be able to differentiate between "root" (e.g. github.com/foo/bar) and "package" portions of import paths, at minimum because the former are governed by hosting platforms and the latter by code authors. doing that is not always possible without network access for go-get metadata, and that's a place the compiler can't go.

but maybe i'm still not understanding, and that's not what you mean?

@sdboyer

yes, i saw that, but that doesn't make sense. the go toolchain doesn't do any kind of version selection right now. that's the problem we're fixing in dep. there is no conceivable way of "fixing" this in the go toolchain, apart from moving the responsibility for version selection into the go toolchain, which is the point of dep in the first place.

No one said we should fix this with versioning. The problem is that the compiler enforces that only a single casing of a package import exists. I am not sure why you think there is no conceivable way of fixing this in Go. Do you have an analysis showing this? Go does import resolution within GOPATH and there may be a possible fix there that doesn't involve a versioning system.
The issue right now is a project and all of its imports need to have the same casing for a package import. If the compiler allowed mixed casing and only raised an error when the package is ambiguous (two casing versions are encountered at the same import priority), we could correct the problem by modifying the set of imports (for example, make sure that only github.com/sdboyer/deptest and not github.com/Sdboyer/deptest), rather than having all of the imported projects agree on a single import path.
Case-folding import paths would not have an effect existing Go code. No one expects github.com/sdboyer/deptest and github.com/Sdboyer/deptest to be different packages. Adding this as a constant factor in the SAT solver seems like a waste, when the package name could be canonicalized to github.com/sdboyer/deptest before adding the case variations to the solution space. When I say this is unrealistic, it means that it doesn't actually help with the problem; all packages have to agree on a single casing of import paths. I am arguing that this shouldn't be a requirement in the first place and making this a requirement is what makes the problem untenable.
Put simply, there may be a solution that exists by moving the enforcement of case variations to the package resolution stage, rather than based on encountered imports. It would then be the problem of dep or some other dependency system (GOPATH, vendor, etc.) to ensure that canonicalized import paths are unique, which is a solvable problem.

I am not sure why you think there is no conceivable way of fixing this in Go. Do you have an analysis showing this?

ahhh, i think i finally see what you're arguing for here. at least, the general shape of it.
my impossibility claim there was based on the assumption that you were talking about doing this via version selection. now that i better understand what you're saying, though, sure - i suspect it's quite possible to do what you're suggesting. it's not clear to me whether it's the right path, though.

two casing versions are encountered at the same import priority

i'm not sure what "import priority" is, but "priority" suggests some concept of ordinality. while we can impose something like that on a DAG with a "shortest/longest path distance from root", it's not clear to me that that's actually a useful distinction for this domain in general, or for this problem in particular. hard to tell though, because:

we could correct the problem by modifying the set of imports (for example, make sure that only github.com/sdboyer/deptest and not github.com/Sdboyer/deptest)

this seems like an incomplete thought? i don't know what the "modification" here is.

making this a requirement is what makes the problem untenable.

at most, i think we can say it makes the problem frustrating. if it were so critically flawed an approach as to be unrealistic, then...well, a bit facetious, but we'd hear about it in TIOBE analyses about Go's viability. what we know is that the check exists in the compiler today, and the actual felt pain from it here, as reflected in this issue, seems to be a confluence of two factors:

logrus is either the or one of the most widely imported public package in the Go ecosystem
Sirupsen chose to change the casing of his name, and people followed suit with their imports

it is, of course, not impossible that this might happen again in the future. but it strikes me as more of an edge cases, having required a bit of a perfect storm to really have bite.

Adding this as a constant factor in the SAT solver seems like a waste, when the package name could be canonicalized to github.com/sdboyer/deptest before adding the case variations to the solution space.

so the proposal is that the compiler only ever sees the folded/canonicalized paths, and that's the information it has when it goes searching on local disk for files to satisfy an import?

Adding this as a constant factor in the SAT solver seems like a waste, when the package name could be canonicalized to github.com/sdboyer/deptest before adding the case variations to the solution space.

so, to be clear, as a practical matter, this has to happen in dep right now, as it's the only solution we have to fix the pain that exists with currently released versions of the toolchain. we can consider taking it out later if a change like you're proposing lands, but it'd probably have to be quite far in the future. dep has to care about a longer span of time than your average project.

@sdboyer That is a closer interpretation of what I'm saying.
I think the problem with the dep solution is that it won't actually be able to solve the problem if dependencies don't agree on case. The situation is no better or worse than it is today, except we might get a better error message.
Either way, let's shift venues. I'm not sure this is the right place to discuss. I haven't found an issue on golang for this, but it is likely that one exists.


Grouping routes for similar functions into seperate files

Hello,
Is it possible to group routes into seperate files the same way you would do it in express?
In express, you could just do:
app.use('/animals', require('./animals'))
and inside animal.js, you could have:
var express = require('express')
  , router = express.Router()

// Domestic animals page
router.get('/domestic', function(req, res) {
  res.send('Cow, Horse, Sheep')
})

// Wild animals page
router.get('/wild', function(req, res) {
  res.send('Wolf, Fox, Eagle')
})

module.exports = router

Such that all the routes are preceded with /animals. Can this be achieved in rill as well?

This is totally possible and works much the same as express.
The big difference is that you can nest subrouters into any rill method (at, get, put, post, etc) and that you have to use a catch all path like '/animals/*'.
Your above example would look like this:
index.js
app.at('/animals/*', require('./animals'))

animals.js
var rill = require('rill')
  , router = rill()

// Domestic animals page
router.get('/domestic', function(rctx) {
  ctx.res.body = 'Cow, Horse, Sheep'
})

// Wild animals page
router.get('/wild', function(ctx) {
  ctx.res.body = 'Wolf, Fox, Eagle'
})

module.exports = router

Feel free to ask more questions if this unclear.
Thanks for your question!

@DylanPiercey So they can't be exactly the same then.

@imolorhe what do you mean? I never claimed for it to be exactly the same as express 😜. Rill itself more similar to koa than express but ultimately it's different than both because it is optimized for both the browser and the server.

@DylanPiercey Okay. How about passing multiple middlewares to app.get() or app.at()?


Better session creation algorithm for selenoid nodes

Codecov Report

Merging #395 into master will decrease coverage by 0.25%.
The diff coverage is 67.74%.

@@            Coverage Diff             @@
##           master     #395      +/-   ##
==========================================
- Coverage   74.57%   74.32%   -0.26%
==========================================
  Files           9        9
  Lines        1070     1071       +1
==========================================
- Hits          798      796       -2
- Misses        222      225       +3
  Partials       50       50

Flag
Coverage Δ

#unittests
74.32% <67.74%> (-0.26%)
:arrow_down:

Impacted Files
Coverage Δ

main.go
67.7% <100%> (+0.52%)
:arrow_up:

protect/queue.go
77.77% <58.33%> (-3%)
:arrow_down:

selenoid.go
76.42% <66.66%> (-0.66%)
:arrow_down:

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 3225edd...9d95095. Read the comment docs.


Contributors update

What:
Updated the contributors info

Codecov Report

Merging #441 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #441   +/-   ##
=======================================
  Coverage   88.35%   88.35%
=======================================
  Files          68       68
  Lines        1013     1013
  Branches       41       41
=======================================
  Hits          895      895
  Misses        100      100
  Partials       18       18

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 46f1c59...7610591. Read the comment docs.

Great!


Improvements for testing

#150

I made a big comment above the new m explaining how to use them.  I'm not sure if dartanalyzer asks you to write a comment for each


Add check if canonical id is already in the system

@sinarz I added an extra check to ypur branch.
feel free to merge this one

Thank you, a really good commit since it stops possible bugs from GCM or wrong/duplicate registrations done by the client.

@matzew Other than the typo looks good.

@secondsun updated
@sinarz can you merge? Than I will be merging your PR against AG tomorrow :tada:

@sinarz @secondsun I added another if, to remove the old/duplicated token in case the canonical ID is already registered

closing, code has been rebased and squashed on https://github.com/aerogear/aerogear-unifiedpush-server/pull/693


Can't send MQTT - Error: Not Found - 404

Hi,
i'm trying to send MQTT messages. It didn't work with my 4.5.6 Version so i updated to 5.1.0.
Now if i hit Test-On or Test-Off a message pops up:
"Request Error, Pleae look in your habridge.log: undefined with status: Not Found - 404"  (and yes the "please" is spelled wrong ;-) )
log says:
The requested route [/(null)] has not been mapped in Spark for Accept: [null]

Any update on this?


Prefer std::size Over In-Place Size Literals

There are a lot of places through-out D2MOO where a statically sized array (normally nested in a struct) is iterated with a for loop, using an in-place literal for the size of that array; In almost all cases these should be replace with std::size.
As an example:
https://github.com/ThePhrozenKeep/D2MOO/blob/e4306096435ec2af96ac36df07c1001f77698a88/source/D2Game/src/QUESTS/ACT5/A5Q5.cpp#L216
This can be replaced with the following:
for (size_t i = 0; i < std::size(pQuestDataEx->bAncienStatueRespawned); ++i)

For those kind of arrays I think the vest would be to use ranged for loops.
In the few places they can't be used (and that I rewrote), I used ARRAY_SIZE because at some point I wanted to be compatible pre-c++17 but I think that ship has sailed? I also wanted to limit usage of the stdlib. (mostly for build times).

Feels weird to want to limit the use of stdlib. People know how (or atleast should learn) to use the stdlib, and prefer stdlib over ugly code/hand rolled macros.
Most objects are built once, even if you are iterating on something.

For those kind of arrays I think the best would be to use ranged for loops.

In D2 there are a lot of loops where the loop index is used for more than just the current array index; where suitable ranged-based seem fine; but std::size is at least a step up from literals and defines.

In the few places they can't be used (and that I rewrote), I used ARRAY_SIZE because at some point I wanted to be compatible pre-c++17 but I think that ship has sailed? I also wanted to limit usage of the stdlib. (mostly for build times).

I mean if thats the case, we can easily make a header with our own templates for array sizes and offsets (which might be preferable since there are cases where std::size doesn't work; eg: when you don't actually have an object) rather than relying on macro's (which have issues). However, std::size is already used in the code base, and on top of that C++17 is ~7 years old by now; what exactly is the gain for supporting pre-C++17 compilers?
Regarding compile times, I don't think single line constexpr templates are going to increase them, at least not by any noticeable amount -- even if they do, I believe the trade-off for both security and modability is worth it (especially on the modability front, increasing an arrays size should just work without need to go analyze every reference). IMO this should be treated the same as the usage of std::min & std::max everywhere.


Request for Feedback on blog post

I started writing this up as a post on the spring-cloud Gitter, but thought it might make more sense to post somewhere a little more permanent.
I recently wrote a blog post about using spring cloud gateway to create a route service.  First off/selfishly, I was wondering if anyone had suggestions on alternate approaches I could/should have taken to implement the route service.
Secondly, I wanted to bring up a couple ergonomic issues and see if they would make sense to create separate issues/contributions for:

Should it be possible to create a route without a URI? This might make sense if the final destination is determined within a filter
Does a single arg header filter make sense? This would just check for the existence of a header...  instead of header("X-My-Header", ".*"), it would be header("X-My-Header")
in GatewayFilterSpec:

Should add and filter both exist? It feels a little weird that they both exist w/ the same functionality
If add and filter are meant to be aliases, should it be possible to specify an order using add?
Should there be a varargs version of addAll? Adding multiple filters is currently a little awkward
Would it make sense to automatically get the order for filters which implement Ordered?

Header existence check added here 9d04c4eca15ffbf3ba0b9662ccae6f6b45a8cdb5
GatewayFilter spec tightened here ee55ef50e71dd8c3f94915083bd39715db6ea17a
I believe ordering is taken advantage of https://github.com/spring-cloud/spring-cloud-gateway/blob/master/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/handler/FilteringWebHandler.java#L83
If not log a separate issue.
So the issue remains about URI. (nice blog post BTW).
I'd propose that we build in support for this. You are not the first to ask, and folks liked your post.
Make RouteServiceForwardingFilter a global filter similar to the ribbon one. Make the uri something similar to, like cf:route. RouteServiceForwardingFilter would only filter if the URI matched. I'd assume all routes would want to match the 3 headers, we could make a specific predicate for those.
Thoughts?

Cool, thanks for the quick updates, and I put in #192 for ordering.
I think Adding in a RouteServiceForwardingFilter is reasonable, it's fairly specialized, but it's not like there's a whole lot of overhead involved.
I feel like the dynamic routing might have use cases beyond just the cf route service, would dynamic:route maybe make sense (or .dynamicUri())?

I think dynamic is too generic since the filter is getting the URL from a cf specific header X-CF-Forwarded-Url. If we make the header configurable, then maybe something else.

So with a global filter to manage the forwarded URL, a route might look something like this?
return builder.routes()
        .route(r ->
                r.predicate(cfRouteServicePredicate)
                        .filters(f -> f.requestRateLimiter(args))
                        .uri("cf:myroutename"))
        .build();

yup, or even r.cfRouteServicePredicate()...

Great! We also use the gateway as CF route service. So I can drop my workaround sooner or later. Thanks

I've sent a PR  so that we can change the url dynamically.
Does adding more specific CloudFoundryRouteServiceFowardGatewayFilterFactory make sense?


Merge configurations

I'd like to discuss how we merge separate levels configurations and how we can improve it.
Problems:

Analyzer settings are not typed, so they cannot be merged - only replace in full.
There is no way to reset any collection/dictionary or remove any value from it - only add additional values to the end.

First problem may be resolved if we'll merge config through reflection or as JSON.
I don't know how we could solve second problem. Any ideas?

Deleting values from lists is tricky. I guess we could define a syntax for it.
Defining strong types for analyzer settings would be fine with me, as would doing it via reflection.

What do you think such syntax:

For dictionary - set any key explicitly to null will remove object - this is really straightforward.
For list: if first element is { "command" : "clear" } or { "command" : "remove", values: [...] } we will process them on merge.

With it, simplest way to merge setting, will be serialize to JSON both source and target, merge them as JSON and do list post-processing.
After it, deseriallize settings back.

null to remove makes sense. some sort of meta value like { "command" : ... } sounds fine too. ideally we do something to make it clear that it's a meta-value, like making it __COMMAND__ or something. No particular impulse there towards what.
We may just want to look at existing textual configuration file formats and see if there's something out there we should replace the json with.


Wrong name used for ODBC Driver

On the page, Add the RD Connection Broker server to the deployment and configure high availability, an example connection string is given:
Driver={SQL Server Native Client 13.0};Server=tcp:cb-sqls1.database.windows.net,1433;Database=CB-DB1;Uid=sqladmin@contoso;Pwd={your_password_here};Encrypt=yes;TrustServerCertificate=no;Connection Timeout=30;

The ODBC driver must be installed on the connection broker server, and the page gives a link to that download: https://www.microsoft.com/en-us/download/confirmation.aspx?id=50420
However, whereas the example connection string uses SQL Server Native Client 13.0 as the name of the driver, it is actually named ODBC Driver 13 for SQL Server when installed. This mismatch causes the connection to fail when setting up high availability of connection brokers.
The page should be updated with the correct name of the driver.

Thank you for being very precise about the details of this issue.
That resolves an unanswered question I had after looking at the same type of issue ticket in Suggestion from customer on the Connection string #4080
I will open a PR to get that string corrected, unless someone else has created a PR already.

(ref. #4080 and #4840 )

PR #4974 is now ready for review. Feel free to browse, comment, or suggest changes.


Add Layer for qemux86

This adds support for running RAUC in an x86 qemu image.
Features:

A+B+recovery system example configuration
GRUB boot source selection
Atomic EFI bootloader (GRUB) updates
Shared data partition (with RAUC central slot status)
Example bundle recipe

@ejoerns wonderful! We have another supported machine :)
Cheers, Leon


Issue #3 replicated in Spavro 1.1.15

I'm able to replicate issue #3  running spavro 1.1.15. I'm running it in pipenv so I'm pretty sure I don't have an out-dated dependency. The only apparent thing that's changed is the line number of the throwing error.
For reproduction, please use the artifact in the original issue.
from Pipfile.lock
        "spavro": {
            "hashes": [
                "sha256:25e2994564df461baf739d2825e2451d5875de5583d0ed8c92070738661a6f4a"
            ],
            "version": "==1.1.15"
        }

TB:
Traceback (most recent call last):
  File "src/spavro/fast_binary.pyx", line 705, in spavro.fast_binary.get_writer
  File "src/spavro/fast_binary.pyx", line 499, in spavro.fast_binary.make_union_writer
  File "src/spavro/fast_binary.pyx", line 363, in spavro.fast_binary.get_check
  File "src/spavro/fast_binary.pyx", line 367, in spavro.fast_binary.make_record_check
  File "src/spavro/fast_binary.pyx", line 363, in spavro.fast_binary.get_check
  File "src/spavro/fast_binary.pyx", line 411, in spavro.fast_binary.make_union_check
  File "src/spavro/fast_binary.pyx", line 363, in spavro.fast_binary.get_check
KeyError: 'org.eha.demo.parents'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./test.py", line 68, in <module>
    test(obj, schema)
  File "./test.py", line 42, in t2
    with DataFileWriter(bytes_writer, DatumWriter(), avsc, codec='deflate') as writer:
  File "/home/sarwar/.local/share/virtualenvs/avro-check-in6pzLTy/lib/python3.5/site-packages/spavro/datafile.py", line 99, in __init__
    self.datum_writer.writers_schema = writers_schema
  File "/home/sarwar/.local/share/virtualenvs/avro-check-in6pzLTy/lib/python3.5/site-packages/spavro/io.py", line 820, in writers_schema
    self.write_datum = get_writer(parsed_writer_schema.to_json())
  File "src/spavro/fast_binary.pyx", line 709, in spavro.fast_binary.get_writer
KeyError: 'union'

Closed due to reopening of #3


Adding a README file to www/swagger-ui with instructions on how to update files in that directory.

Closing this request since https://github.com/GoogleCloudPlatform/kubernetes/pull/3232 takes care of this.


Environment variables

- Summary
The environmental variables inside the gulp tasks make it easy to use them within theme files. It's now easy to have switch between environments where you might need another base-path (like for github pages) or maybe include a script for debugging.
You can query the environment variables within the theme with getenv "NODE_ENV" - it will return development for all hugo* tasks and production for all build* tasks.
I also fixed two linting errors, added some comments to the gulpfile and updated the Readme to include the latest additions.
- Test plan
I've added {{ getenv "NODE_ENV" }} to site\layouts\index.html and run gulp hugo, gulp hugo-preview and gulp server which all three would yield development in the resulting file. While gulp build and gulp build-preview wrote production to the created files. Same would work with the npm scripts.
- Description for the changelog
Added environment variables inside gulp tasks
- A picture of a cute animal (not mandatory but encouraged)

Nice, thanks a lot @atomtigerzoo 🙌
LGTM


Refactor App to use react-router

Heya there,
This is a very late PR to address #48, which looks to enable react-router based navigation. Turns out, this is slightly trickier than I thought it'd be, but I believe I have a working copy now. Along the way, I did some other minor refactoring to make things work as intended.
Here's the cliff's notes of the changes:

adds react-router-dom to our dependency list
adds an encompassing <Router> to our index.js; moves <Layout> from <App> to a similar position. This is required to stop layout refreshes on App state changes.
renames lessons to activities
adds a <Switch> to App.js, with several key routes:

/, which goes to the Landing Page
/activity, which goes to the first activity
/activity/:activitynum, which goes to the :activitynumth activity (1-indexed, e.g. /activity/2 is the second activity`
/end, which goes to the closing tile
/game, which goes to the game section (not done implementing yet, should punt to another PR)

adds a Nav to the entire app, so users can easily navigate between the activity and game; pads out top of the page to account for space
changes the behaviour of <Main> and <LessonText> to use functional change lesson buttons, which use the useHistory hook. This also lets us remove setLessonNum and setLessonAndCount, as the browser URL becomes the source of truth
adds more info to page title
removes all id tags
removes some overrides for consistent styling rules
in anticipation for client-side routing, adds a public/_redirects file for Netlify

There is still quite a bit of layout polish to do, though I believe I've covered everything directly related to react router. Anyways, since this was a lot of code, it's very likely that I slipped up - please let me know if you see something!
Closes #48.
Preview: https://deploy-preview-54--stupefied-varahamihira-55e231.netlify.app/

Hey Matt, thanks for cranking out the refactor! I just gave it a look and things look good. Not too much to comment on the code, but a small detail I noticed was that you kept activities one and two separate, but joined activities three and four. Any particular reason?
Another note: while I do think that the navbar is a nice addition since the game will be coming along, it definitely gives off some chunkier vibes that makes the experience a little stranger on iPads. The scrolling is also a little hmm. This is a small feature, but just wanted to get some input.

Going to merge rn out of need for speed but can you elaborate on the iPad thing? Like is it a Safari issue?

Don't worry about it. Pertained to UX but is largely inconsequential.


Incorrect Color

When I run a test using Vibrant.js on just a single solid color jpeg, the output color is incorrect.
Test was performed using this Fiddle: http://jsfiddle.net/oskar/3479gj9e/light/
Color used for test:
Hex #313788
Output Colors:
Circle 1:
Hex #343c8c
Circle 3:
Hex #383c8c

This is because Vibrant (just like it's Palette android equivalent) will try to generate colors (to be specific, either VibrantSwatch or DarkVibrantSwatch) when they don't exist.
Vibrant was mostly made for photographs aka things with a lot of different colors in them. You can't really blame it for not picking up a single color. I suggest looking into ColorThief (on which vibrant was based) to get a palette / the most used color.
TL;DR: It's a feature, not a bug.


Changes of Catalog in a project

If the questionnaire is changed for an active project, the stored information is presented with the new questionnaire

in snapshots
in xml exports
If one wants to preserve both together, this should be offered as an option (e.g.: as a complete export)

This is a bit more tricky to archive, if the old catalog is still there, the user could just switch back. If we want to store the information about the actual question answered, we would need to store it in the Value or have even a bigger change in the RDMO data model.

If just the whole catalog should be added to the export for long-time archival purposes, its relatively simple.


Add button to collapse all sections of the page at once

As of Wagtail 4.0, all panels / StreamField blocks / InlinePanel child items are collapsible, all using the same "panel" component. The next step is to add a "collapse all" button, which collapses all of those panels at once.
View the proposed designs in Figma. Screenshot:

Open questions

[ ] The designs were made with the page editor minimap in mind. TBC where we place the button since the minimap hasn’t been implemented yet
[ ] TBC which views exactly we add this to – pages, snippets, etc.
[ ] Upon activation, does the button turn into "Expand all"?
[ ] Is the wording in the designs confirmed? Technically some of those collapsible sections aren’t fields – "Collapse all sections"? "Collapse all panels"?

Implementation plan
Here is the existing toggleCollapsiblePanel logic. The panels and their respective toggles should be updated in the same way with the "collapse all" feature.
For starters, we would most likely add this to pages create.html and edit.html views.

Initialising of the button should either be done in initCollapsiblePanels, or in a separate function called similarly
Button styles for reference: https://github.com/wagtail/wagtail/blob/aa70a9f5bd47610fb199b1bb7cf5e0ad5d39c9bc/wagtail/admin/templates/wagtailadmin/widgets/chooser.html#L39

Test plan

[ ] Check what happens with events firing multiple times as all sections collapse (commentAnchorVisibilityChange)

Further questions / issues to resolve:

Is it correct that the aria-controls for the expand / collapse all button should list the ids of all the panels (div element with class w-panel__content)? If so, I may need some advice on getting these ids from the parent template (edit.html). Potentially they could be added with JavaScript if it is hard to add them in the template.
When we collapse all, should any fields containing errors remain open?
For now, the collapse all / open all is calling the toggleCollapsiblePanel function for each toggle - this means that the events this function fires are sent multiple times - to check if this is an issue for comments
Should we fire another custom event to say that all panels have been collapsed / expanded

Draft MR to show work in progress: https://github.com/wagtail/wagtail/pull/9167

@thibaudcolas I think it is all behaving as expected with comments, but would appreciate you testing that as I'm less familiar with the expected bevaviour.


Support for #[Deprecated()]  annotation

phpstorm-stubs use JetBrains\PhpStorm\Deprecated annotation to mark method's, class e.t.c
This annotation allows for more detailed description of the problem and suggested changes.
Other projects use this annotation as well
Can  deprecation-rules show it in the same way like @deprecated ?

Could be a nice feature!
The today work around: Double @deprecated and #[Deprecated]
/**
 * @deprecated
 */
#[Deprecated("use createFromDateRange() instead", 'DateHelper::createFromDateRange(%parametersList%)')]
public static function fromDateRangePicker(string $dateRangeFormData): array
{
    return self::createFromDateRange($dateRangeFormData);
}

It's not hard at all. The issue is that this attribute isn't official in any way. Someone might prefer to use a different attribute (like their own class) for this same purpose so we better make sure this system is extensible like other similar extension types in PHPStan: https://phpstan.org/developing-extensions/extension-types

I think this would be achievable if \PHPStan\Rules\Deprecations\DeprecatedScopeHelper was aware of the node in \PHPStan\Rules\Deprecations\DeprecatedScopeHelper::isScopeDeprecated.
A custom deprecated scope resolver could then inspect the node to see if it has the attribute.
The problem is I cannot think of a non-breaking way to introduce a change to \PHPStan\Rules\Deprecations\DeprecatedScopeResolver::isScopeDeprecated

the native attribute with php 8.4 which was added with https://wiki.php.net/rfc/deprecated_attribute already works like expected

@staabm It doesn't. I have about 500 LoC commit in my private PHPStan 2.1.x branch that adds proper support for that to PHPStan.
Hopefully it's going to be out still in 2024.

in my local testing it at least reports calls to functions annotated with #[\Deprecated].
will wait for your work then so we don't step onto each others toe