🇺🇸United States @smustgrave

Account created on 30 June 2015, over 9 years ago
  • Software Engineer at Mobomo 
#

Merge Requests

More

Recent comments

🇺🇸United States smustgrave

Fixed the layout_options/new grid patterns in 1 issue
the ui_example in another

and the combo box here. Not thrilled with it but that's my fault for being eager with the patterns for forms.

🇺🇸United States smustgrave

For the help in slack

🇺🇸United States smustgrave

With D7 shutting down in a few weeks I'm triaging the D7 queue of scanner.
Will leave bugs open for a bit longer and maybe, if the other committers want
to, do a final D7 version release of scanner

🇺🇸United States smustgrave

With D7 shutting down in a few weeks I'm triaging the D7 queue of scanner.
Will leave bugs open for a bit longer and maybe, if the other committers want
to, do a final D7 version release of scanner

🇺🇸United States smustgrave

So just joined the scanner team and unfortunately not sure anyone has the time or is working on backdrop to be able to assist. But feel free to use anything here to help!

🇺🇸United States smustgrave

8.x-1.x pipeline is green and manual testing seems good.

If no objections will tag a 8.x-1.0 release and 2.0.x for D11.

🇺🇸United States smustgrave

Wonder if still experiencing this with latest drupal and scanner versions?

🇺🇸United States smustgrave

Started triaging the D7 queue here. Closing most feature requests. If the bug fixes are simple maybe we include but wouldn't worry much abotu D7 anymore.

🇺🇸United States smustgrave

With D7 shutting down in a few weeks I'm triaging the D7 queue of scanner. Will leave bugs open for a bit longer and maybe, if the other committers want to, do a final D7 version release of scanner

🇺🇸United States smustgrave

With D7 shutting down in a few weeks I'm triaging the D7 queue of scanner. Will leave bugs open for a bit longer and maybe, if the other committers want to, do a final D7 version release of scanner

🇺🇸United States smustgrave

Wonder if still needed for the D10+ branches?

🇺🇸United States smustgrave

With D7 shutting down in a few weeks I'm triaging the D7 queue of scanner. Will leave bugs open for a bit longer and maybe, if the other committers want to, do a final D7 version release of scanner

🇺🇸United States smustgrave

Should this be bumped to the 2.0.x branch (when I create it)

🇺🇸United States smustgrave

With D7 shutting down in a few weeks I'm triaging the D7 queue of scanner. Will leave bugs open for a bit longer and maybe, if the other committers want to, do a final D7 version release of scanner

🇺🇸United States smustgrave

Since there hasn't been a follow up to #3 going to close as outdated. If still a separate issue from the mentioned one please reopen.

🇺🇸United States smustgrave

Just joined the scanner team! Going to commit this for a stable release for 8.x-1.x then will plan for D11 :)

🇺🇸United States smustgrave

Looks like a d11 release was made and help not needed :)

🇺🇸United States smustgrave

According to upgrade_status these are the only 2 items needed

🇺🇸United States smustgrave

Possible to get a tagged release of this module please.

🇺🇸United States smustgrave

I was half way through updating and realized this module is in core now haha. I marked it obsolete. Think I'm going to move the 1 feature request https://www.drupal.org/project/file_delete/issues/3111440 to it's own module.

🇺🇸United States smustgrave

Thanks for opening! So the functionality of this module actually landed in 10.1 so this module is no obsolete.

🇺🇸United States smustgrave

With the major function of this module now in core think this module is probably going to be marked obsolete.

🇺🇸United States smustgrave

Fixes should be in MRs but the major functionality of this module is actually in core.

🇺🇸United States smustgrave

So going to do a D11 release but really this module is now obsolete as of D10.1 🐛 There is no way to delete file entities of other users Fixed

🇺🇸United States smustgrave

Believe this to be out of scope but going to link to the module in #3

🇺🇸United States smustgrave

Linked issue has landed so this module may be obsolete but updated module page anyway.

🇺🇸United States smustgrave

Pipeline has errors. Would recommend verifying that everything passes before putting into review please.

🇺🇸United States smustgrave

Change looks good. Mind filling out the issue summary though for the TBD sections. Will keep an eye out for this again.

🇺🇸United States smustgrave

Seems the title update and issue summary update are still needed.

Also fixes should be in MRs vs patches.

🇺🇸United States smustgrave

For all the help in slack.

🇺🇸United States smustgrave

Searched for Drupal CI and all 4 instances have been replaced and the MR appears to have good replacements for each wording.

🇺🇸United States smustgrave

Believe feedback has been addressed.

🇺🇸United States smustgrave

Can say they probably won't go for the hardcoded px approach when there was an effort to move to newer properties like margin-block, as an example

🇺🇸United States smustgrave

I'm seeing several issues related to using Olivero as an admin theme. it was never meant to be used that way.

@saurav-drupal-dev could you explain the use-case a bit more? I'd rather not have people spend time on making Olivero work as an admin theme, and instead work on Claro or Gin

🇺🇸United States smustgrave

Just some small comments on the MR.

🇺🇸United States smustgrave

Believe the 115 should be fine.

Saving credit for shalini_jha for the continued work on it and debugging.

🇺🇸United States smustgrave

I think it would be good to consider maybe upping? I really get both sides

📌 Move helpers in contact_storage_test.module and delete it Active this is a piece of a meta converting all test modules to hooks. The fact it was broken out to this smaller one made it super quick to review. If the developer tried to group say 6 modules of tests to convert that could take a little longer. And this may be a bad example but could hold up good to go pieces from getting in.

So I'd be +1 for upping it slightly but also recognizing and suggesting that breaking things up is always the fastest approach.

🇺🇸United States smustgrave

Seems like a good conversion. Checked the field module and only test module that needed to be updated. LGTM

🇺🇸United States smustgrave

If that's the solution we are going to go with mind updating IS proposed solution section please.

🇺🇸United States smustgrave

Seems pretty straight forward.

Seems to be the only test module in contact that needed the updating

🇺🇸United States smustgrave

Seems pretty straight forward.

I rebased the MR because I couldn't re-run, but the build step had failed which I'm hoping isn't a new random issue popping up. But rebase based now just our regular set of random failures.

🇺🇸United States smustgrave

With last nights commits appears to need a rebase.

if you are another contributor eager to jump in, please allow the original poster @nicxvan at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

🇺🇸United States smustgrave

Doesn't appear to be a bug but more a feature request.

Probably will need submaintainer sign off and usability review.

🇺🇸United States smustgrave

Thanks for reporting, will need a test case showing the issue as a next step.

🇺🇸United States smustgrave

lets add a test though for the issue I saw in #21. Making sure when you click a link it filters and when you click 'All' it get all options

Left a comment on the MR.

🇺🇸United States smustgrave

May be something good to have test coverage for

🇺🇸United States smustgrave

at this point kinda feels like a feature request vs an actual bug. Since this isn't technically breaking anything.

🇺🇸United States smustgrave

Out of curiosity does this solve your issue? https://www.drupal.org/project/better_exposed_filters/issues/3379736 🐛 Collapsible details element not hidden if "Hide filter, if no options" is selected. Active

🇺🇸United States smustgrave

Tried replicating on latest 7.0.x

Using the bef_test module
Edited bef_integer to use links
Ajax is enabled
Go to the view page
Click one of the links and it filters just fine.

Any additional steps to provide/

🇺🇸United States smustgrave

I have 0 idea how to test this with just in bef but if it works with facets I can merge it.

🇺🇸United States smustgrave

Do you have Auto submitted selected in the settings?

🇺🇸United States smustgrave

So is this a facets issue?

I enabled the bef_test module which has a view with an exposed block.
I placed the block on every page
I visited some gibberish url
Got the 404 page not found and my block just fine.

7.0.x and 11.1.x

Sorry if I'm missing something just trying to replicate so I can merge and add test coverage.

🇺🇸United States smustgrave

Wonder if anyone mind testing if this works for them? Then can add coverage for the other params

🇺🇸United States smustgrave

Fine to commit this as is as I found another issue that will require test coverage.

🇺🇸United States smustgrave

Any potential solutions would be welcome

🇺🇸United States smustgrave

So I tried replicating this but not getting an error, could some one verify if it still happens on 7.0.x?

🇺🇸United States smustgrave

Need additional steps

1. Does this happen on 7.0.x

Currently I can't replicate.

🇺🇸United States smustgrave

Wasn't able to reproduce on clean install of 7.0.x

Radio buttons and checkboxes both reset just fine.

🇺🇸United States smustgrave

So when I add the bef_location field to the test view and apply the MR

I do see the -Any- option
But when I try it I get "The submitted value All in the bef_location (field_bef_location) element is not allowed."

🇺🇸United States smustgrave

Steps seem pretty clear to add a simple test case or additional assertion.

Since the change was done without question @deepali sardana to receive credit please turn to an MR and add test coverage

Production build 0.71.5 2024