-
-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change to master isn't shown in pull request status #182
Comments
Yep , that's a thing I'm not super happy about. Do you have suggestions on how we can do better in the case of multiple files? (P.S. I'm going to make the issue description a bit tinier to get the feature request across, I hope you don't mind) |
We're experimenting with this on a fork, but it requires additional permissions (commenting on PRs). There's just not enough room on build status. |
that's what I struggled with 😔 |
@siddharthkp no problem, original point comes across very well with your edits! @threehams In our case we're checking the size of four files. Having all of those featured in the status is indeed not feasible due to space restrictions. If comments require additional permissions, that'd probably be suboptimal indeed. But, what about the next two suggestions? One status for each file: Combine statistics of all checked files in the message: I'd definitely be willing to help out with either of these. Although the first has my preference, the second is already a very good improvement. There's one downside with the second version, though. For example, if we have a file A with a limit of 10 and B with a limit of 100. That's 110 total. But what if the actual size of A is 20 (twice the limit) while B is 50? That'd be displayed as 70/110, yet it would fail. That may cause some confusion. |
I really like idea number 2! the first one is more explicit but also more noisy 😢 One tiny optimisation that can make no. 1 better would be to skip the files that have no change. Personally, I think if there's only one bundle that's effected, we should show that bundle and leave the rest to the details page. If it's more than 1, switch to idea no. 2 |
Skipping files with no change would not allow repositories to mark a status check as required, which is a bit of a bummer as that way you can't force your co-workers to keep it within limits 👼
Yeah, I agree. Perhaps this could be the behavior for different situations:
Perhaps the messages can be a bit cleaner, but I think it'd work. Edit: Looking at the code I can have this done by tomorrow. Since this change would also benefit the company I work for, they won't have any issue with me working on this. |
I like the idea of multiple statuses, but it would definitely need to be configurable. We're a bit edge case in that we'll (eventually) have over 1000 individual files to size-check, so that'd be a bit much. |
@StephanBijzitter go for it! |
@threehams For now though, I'll focus on getting better messages in one status for multiple files. I've tested my initial changes against our project and the messages work correctly, but somehow (even though I can see the change compared to master in the status message) the details page doesn't show the values for master anymore. But hey, progress is being made! |
@siddharthkp Could you take a look at #183, please? The messages are working correctly, but there's two issues that I don't understand. I've listed these in the description of the pull request. I'm hoping someone with (much) more experience in this project (you) will be able to see where I messed up. |
Sounds good, will take care of it |
Being able to see the change is very motivating, especially when you're trying to reduce it.
Currently, I can only see the change when clicking on the 'Details' link (The status message is not very helpful)
What can we improve to make the default message better/communicate changes upfront?
Original issue description is folder here:
Do you want to request a feature or report a bug?
A bug, although it may be a feature request if this is intentional.
What is the current behavior?
![screen shot 2017-11-09 at 17 34 45](https://user-images.githubusercontent.com/1649903/32617306-f6a43e44-c574-11e7-9091-295216e66f99.png)
Currently, my pull request tells me that I've done a good job and that my bundles are within limits.
If the current behavior is a bug, please provide the steps to reproduce.
I have no idea, but here is my configuration:
What is the expected behavior?
![screen shot 2017-11-09 at 17 36 00](https://user-images.githubusercontent.com/1649903/32617314-fc566aa6-c574-11e7-8960-73039c5c4a03.png)
My pull request doesn't just tell me I did a good job, but also tells me the change in size.
If this is a feature request, what is motivation or use case for changing the behavior?
Being able to see the change is very motivating, especially when you're trying to reduce it.
Currently, I can only see the change when clicking on the 'Details' link.
Please mention other relevant information.
The text was updated successfully, but these errors were encountered: